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

Denis Kenzior <[email protected]>
Newsgroups dev.linux.lists.ofono
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.