Hi Bastien,
On Mon, Aug 5, 2024 at 10:09 AM Bastien Nocera <hadess@xxxxxxxxxx> wrote:
>
> Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important]
> bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted.
> bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed.
> bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)".
> 1684|
> 1685| while (sent < size) {
> 1686|-> int n = send(session->sock, buf + sent, size - sent, 0);
> 1687| if (n < 0)
> 1688| return -1;
> ---
> lib/sdp.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/sdp.c b/lib/sdp.c
> index 411a95b8a7d3..8a15ad803db1 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -1678,13 +1678,13 @@ sdp_data_t *sdp_data_get(const sdp_record_t *rec, uint16_t attrId)
> return NULL;
> }
>
> -static int sdp_send_req(sdp_session_t *session, uint8_t *buf, uint32_t size)
> +static int sdp_send_req(sdp_session_t *session, uint8_t *buf, size_t size)
> {
> - uint32_t sent = 0;
> + size_t sent = 0;
>
> while (sent < size) {
> int n = send(session->sock, buf + sent, size - sent, 0);
> - if (n < 0)
> + if (n < 0 || sent > SIZE_MAX - n)
> return -1;
Not really following you here, it seems the problem is that sent + n
could overflow causing sent to loop around but if that happens don't
we get a problem with send syscall itself? Using size_t makes sense
but I guess we want n to also be ssize_t then, and perhap use if (n <
0 || n > size - sent) logic if we cannot trust syscalls returns the
correct number of bytes.
> sent += n;
> }
> --
> 2.45.2
>
>
--
Luiz Augusto von Dentz
