diff mbox series

smsutil: check that user data length fits in internal buffer

Message ID 20240809111213.853665-1-j.lemetayer@kerlink.fr (mailing list archive)
State Superseded
Headers show
Series smsutil: check that user data length fits in internal buffer | expand

Commit Message

Jean-Marie Lemetayer Aug. 9, 2024, 11:12 a.m. UTC
This addresses CVE-2023-2794.
---
 src/smsutil.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Denis Kenzior Aug. 9, 2024, 3:27 p.m. UTC | #1
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
Jean-Marie Lemetayer Aug. 9, 2024, 3:39 p.m. UTC | #2
Hi Denis,

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

I will provide a v2.

Best regards,
Jean-Marie
diff mbox series

Patch

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;
+
 	memcpy(out->deliver.ud, pdu + offset, expected);
 
 	return TRUE;