Hi Bastien, On Mon, Aug 5, 2024 at 10:09 AM Bastien Nocera <hadess@xxxxxxxxxx> wrote: > > Error: INTEGER_OVERFLOW (CWE-190): [#def4] [important] > bluez-5.77/monitor/control.c:1094:2: tainted_data_return: Called function "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)", and a possible return value may be less than zero. > bluez-5.77/monitor/control.c:1094:2: assign: Assigning: "len" = "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)". > bluez-5.77/monitor/control.c:1099:2: overflow: The expression "data->offset" is considered to have possibly overflowed. > bluez-5.77/monitor/control.c:1115:3: overflow: The expression "data->offset -= pktlen + 6" is deemed overflowed because at least one of its arguments has overflowed. > bluez-5.77/monitor/control.c:1118:4: overflow_sink: "data->offset", which might have underflowed, is passed to "memmove(data->buf, data->buf + 6 + pktlen, data->offset)". [Note: The source code implementation of the function has been overridden by a builtin model.] > 1116| > 1117| if (data->offset > 0) > 1118|-> memmove(data->buf, data->buf + MGMT_HDR_SIZE + pktlen, > 1119| data->offset); > 1120| } > --- > monitor/control.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/monitor/control.c b/monitor/control.c > index 009cf15209f0..62857b4b84de 100644 > --- a/monitor/control.c > +++ b/monitor/control.c > @@ -18,6 +18,7 @@ > #include <stdbool.h> > #include <stddef.h> > #include <errno.h> > +#include <limits.h> > #include <unistd.h> > #include <stdlib.h> > #include <string.h> > @@ -1091,9 +1092,14 @@ static void client_callback(int fd, uint32_t events, void *user_data) > return; > } > > + if (sizeof(data->buf) <= data->offset) > + return; > + > len = recv(data->fd, data->buf + data->offset, > sizeof(data->buf) - data->offset, MSG_DONTWAIT); > - if (len < 0) > + if (len < 0 || > + len > UINT16_MAX || > + UINT16_MAX - data->offset > len) > return; Not really sure why we would be using UINT16_MAX here instead of BTSNOOP_MAX_PACKET_SIZE or sizeof(data->buf), and perhaps we should really check that len > sizeof(data->buf) - data->offset since that implicitly guarantees data->offset + len <= sizeof(data->buf), anyway this is again suggesting that a syscall can return a bigger value than it was asked for. > data->offset += len; > -- > 2.45.2 > > -- Luiz Augusto von Dentz
