Re: [BlueZ 11/12] tools/mesh: Fix integer overflow due to cast operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sent from my iPhone

> On Jul 4, 2024, at 12:51 PM, Bastien Nocera <hadess@xxxxxxxxxx> wrote:
> 
> On Thu, 2024-07-04 at 11:45 -0700, Brian Gix wrote:
>> 
>>> On Jul 4, 2024, at 3:27 AM, Bastien Nocera <hadess@xxxxxxxxxx>
>>> wrote:
>>> 
>>> Error: INTEGER_OVERFLOW (CWE-190): [#def29] [important]
>>> bluez-5.76/tools/mesh/mesh-db.c:551:3: cast_overflow: Truncation
>>> due to cast operation on "ele_cnt" from 32 to 8 bits.
>>> bluez-5.76/tools/mesh/mesh-db.c:551:3: overflow_sink: "ele_cnt",
>>> which might have overflowed, is passed to "remote_add_node((uint8_t
>>> const *)uuid, unicast, ele_cnt, key_idx)".
>>> 549|            continue;
>>> 550|
>>> 551|->        remote_add_node((const uint8_t *)uuid, unicast,
>>> ele_cnt,
>>> 552|                                key_idx);
>>> 553|        for (j = 1; j < key_cnt; j++) {
>>> ---
>>> tools/mesh/mesh-db.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/tools/mesh/mesh-db.c b/tools/mesh/mesh-db.c
>>> index 1d047691d240..abcc09d523a5 100644
>>> --- a/tools/mesh/mesh-db.c
>>> +++ b/tools/mesh/mesh-db.c
>>> @@ -503,7 +503,8 @@ static void load_remotes(json_object *jcfg)
>>>        uint8_t uuid[16];
>>>        uint16_t unicast, key_idx;
>>>        const char *str;
>>> -        int ele_cnt, key_cnt;
>>> +        uint8_t ele_cnt;
>>> +        int key_cnt;
>>>        int j;
>>> 
>>>        jnode = json_object_array_get_idx(jnodes, i);
>>> @@ -533,9 +534,6 @@ static void load_remotes(json_object *jcfg)
>>> 
>>>        ele_cnt = json_object_array_length(jarray);
>>> 
>>> -        if (ele_cnt > MAX_ELE_COUNT)
>>> -            continue;
>>> -
>> 
>> What happens if the json file is corrupted and there are more than
>> 255 elements in the array?
> 
> ele_cnt is a uint8_t, so it will wrap around.
> 
> We could add that if you preferred (I checked, and the array length is
> cached):
> 
> diff --git a/tools/mesh/mesh-db.c b/tools/mesh/mesh-db.c
> index abcc09d523a5..4c74e874986c 100644
> --- a/tools/mesh/mesh-db.c
> +++ b/tools/mesh/mesh-db.c
> @@ -529,7 +529,8 @@ static void load_remotes(json_object *jcfg)
>                        continue;
> 
>                json_object_object_get_ex(jnode, "elements", &jarray);
> -               if (!jarray || json_object_get_type(jarray) != json_type_array)
> +               if (!jarray || json_object_get_type(jarray) != json_type_array ||
> +                   json_object_array_length(jarray) > MAX_ELE_COUNT)
>                        continue;

I’m ok with this addition. There is a hard limit on the number of elements in a node, and this edit appears to restore that check.

> 
>                ele_cnt = json_object_array_length(jarray);
> 
> 
> 
>> 
>>>        json_object_object_get_ex(jnode, "netKeys", &jarray);
>>>        if (!jarray || json_object_get_type(jarray) !=
>>> json_type_array)
>>>            continue;
>>> --
>>> 2.45.2
>>> 
>>> 
> 





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux