RE: [PATCH] smsutil: check that user data length fits in internal buffer

Hi Denis,

Sorry for that. With my local GCC I didn't see anything...

I will provide a v2.

Best regards,
Jean-Marie

________________________________________
De : Denis Kenzior <[email protected]>
Envoyé : vendredi 9 août 2024 17:27
À : Jean-Marie LEMETAYER; [email protected]
Objet : Re: [PATCH] smsutil: check that user data length fits in internal buffer

Hi Jean-Marie,

On 8/9/24 6:12 AM, Jean-Marie Lemetayer wrote:
> This addresses CVE-2023-2794.
> ---
>   src/smsutil.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 39f0ecc6..7f065d9d 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -773,6 +773,9 @@ static gboolean decode_deliver(const unsigned char *pdu, int len,
>       if ((len - offset) < expected)
>               return FALSE;
>
> +     if (expected > sizeof(out->deliver.ud))
> +             return FALSE;
> +

This doesn't compile cleanly:

Fedora (glibc) clang optimized
==============================
Configure: PASS
Build: FAIL
     src/smsutil.c:776:15: error: comparison of integers of different signs:
'int' and 'unsigned long' [-Werror,-Wsign-compare]
       776 |         if (expected > sizeof(out->deliver.ud))
           |             ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~
     1 error generated.
     make[1]: *** [Makefile:4073: src/smsutil.o] Error 1
     make[1]: Target 'all-am' not remade because of errors.
     make: *** [Makefile:2383: all] Error 2

Fedora (glibc) clang debug
==========================
Configure: PASS
Build: FAIL
     src/smsutil.c:776:15: error: comparison of integers of different signs:
'int' and 'unsigned long' [-Werror,-Wsign-compare]
       776 |         if (expected > sizeof(out->deliver.ud))
           |             ~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~
     1 error generated.
     make[1]: *** [Makefile:4073: src/smsutil.o] Error 1
     make[1]: Target 'all-am' not remade because of errors.
     make: *** [Makefile:2383: all] Error 2

Alpine (musl) gcc debug
=======================
Configure: PASS
Build: FAIL
     src/smsutil.c: In function 'decode_deliver':
     src/smsutil.c:776:22: error: comparison of integer expressions of different
signedness: 'int' and 'long unsigned int' [-Werror=sign-compare]
       776 |         if (expected > sizeof(out->deliver.ud))
           |                      ^
     cc1: all warnings being treated as errors
     make[1]: *** [Makefile:4073: src/smsutil.o] Error 1
     make[1]: Target 'all-am' not remade because of errors.
     make: *** [Makefile:2383: all] Error 2

Regards,
Denis