Bluetooth: services: Adds additonal len checks for ots_dir_list #33872
Conversation
| __ASSERT((len + offset) <= dir_list->net_buf.len, | ||
| "Invalid dir_list buf length %u, expected at least %u", | ||
| dir_list->net_buf.len, len + offset); |
There was a problem hiding this comment.
Does dir_list.net_buf originate from some remote peer device? If so, then assert is the wrong choice here. That should only be used to catch local programming bugs, whereas invalid data from a peer should be handled gracefully.
There was a problem hiding this comment.
The dir_list.net_buf (in fact everything related to the dir_list) is created by the OTS service locally. It is an internal read-only object.
The dir_list object is, however, created based on metadata from other, potentially remotely created objects, but my opinion is that it is the responsibility of the OTS implementation (and not the OTS dir list implementation) to validate remotely created objects. That being said, the current version of our OTS implementation does not allow a client to create objects, but only the local application.
|
@kapi-no can you please review? |
| net_buf_simple_add_u8(net_buf, strlen(obj->metadata.name)); | ||
| obj_name_len = strlen(obj->metadata.name); | ||
| __ASSERT(obj_name_len >= 0 && obj_name_len <= 120, | ||
| "Dir list object len is too small %u", len); |
There was a problem hiding this comment.
| "Dir list object len is too small %u", len); | |
| "Dir list object len is incorrect %u", len); |
It can be too large.
The first condition seems always true:
obj_name_len >= 0
There was a problem hiding this comment.
The first condition should have been obj_name_len > 0 as empty strings are not allowed for the names.
Adds additional length checks for the OTS directory listing implementation. This will check the object name length and the total length of the object when encoding, as well as the length of the objects when removing objects from the directory listing. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Adds name length checks. The OTS spec does not explicitely specifiy a maximum name length, but the maximum name length in the directory listing object shall be less or equal to 120 octets. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Thalley
force-pushed
the
ots_length_checks
branch
from
af9ac0f to
72e5fc6
Compare
|
@jhedberg Added another commit w.r.t. name length checks. Will you please re-review? |
Adds additional length checks for the OTS directory listing
implementation. This will check the object name length and
the total length of the object when encoding,
as well as the length of the objects when removing objects
from the directory listing.
Signed-off-by: Emil Gydesen emil.gydesen@nordicsemi.no
fixes #33830