Re: [BlueZ v2 01/11] gatt-server: Don't allocate negative data

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

 



On Thu, 2024-07-18 at 14:40 +0200, Bastien Nocera wrote:
> On Mon, 2024-07-08 at 10:44 -0400, Luiz Augusto von Dentz wrote:
> > Hi Bastien,
> > 
> > On Fri, Jul 5, 2024 at 5:00 AM Bastien Nocera <hadess@xxxxxxxxxx>
> > wrote:
> > > 
> > > Set a lower-bound to the data MTU to avoid allocating -1 elements
> > > if
> > > bt_att_get_mtu() returns zero.
> > > 
> > > Error: OVERRUN (CWE-119): [#def36] [important]
> > > bluez-5.76/src/shared/gatt-server.c:1121:2: zero_return: Function
> > > call "bt_att_get_mtu(server->att)" returns 0.
> > > bluez-5.76/src/shared/gatt-server.c:1121:2: assignment:
> > > Assigning:
> > > "data->mtu" = "bt_att_get_mtu(server->att)". The value of "data-
> > > > mtu" is now 0.
> > 
> > We are disconnected or have an invalid bt_att instance if
> > bt_att_get_mtu returns 0 so it is probably pointless to attempt to
> > send any response if that is the case.
> 
> Same as for patch 8 in this series:
> gatt-server: Fix integer overflow due to cast operation
> 
> Is "1" a valid value here?

Never mind, Coverity is happy, so I'm happy too.

> 
> > 
> > > bluez-5.76/src/shared/gatt-server.c:1123:19: assignment:
> > > Assigning:
> > > "__n" = "(size_t)(data->mtu - 1UL)". The value of "__n" is now
> > > 18446744073709551615.
> > > bluez-5.76/src/shared/gatt-server.c:1123:19: assignment:
> > > Assigning:
> > > "__s" = "1UL".
> > > bluez-5.76/src/shared/gatt-server.c:1123:19: overrun-buffer-arg:
> > > Calling "memset" with "__p" and "__n * __s" is suspicious because
> > > of the very large index, 18446744073709551615. The index may be
> > > due
> > > to a negative parameter being interpreted as unsigned. [Note: The
> > > source code implementation of the function has been overridden by
> > > a
> > > builtin model.]
> > > 1121|           data->mtu = bt_att_get_mtu(server->att);
> > > 1122|           data->length = 0;
> > > 1123|->         data->rsp_data = new0(uint8_t, data->mtu - 1);
> > > 1124|
> > > 1125|           return data;
> > > ---
> > >  src/shared/gatt-server.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> > > index 3a53d5dfde6b..66e370d1fe3d 100644
> > > --- a/src/shared/gatt-server.c
> > > +++ b/src/shared/gatt-server.c
> > > @@ -1120,7 +1120,7 @@ static struct read_mult_data
> > > *read_mult_data_new(struct bt_gatt_server *server,
> > >         data->cur_handle = 0;
> > >         data->mtu = bt_att_get_mtu(server->att);
> > >         data->length = 0;
> > > -       data->rsp_data = new0(uint8_t, data->mtu - 1);
> > > +       data->rsp_data = new0(uint8_t, MAX(data->mtu, 1) - 1);
> > > 
> > >         return data;
> > >  }
> > > --
> > > 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