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 > > > > > > > > > > >
