[PATCH 1/2] smsutil: check deliver reports fit in buffer

[PATCH 1/2] smsutil: check deliver reports fit in buffer

[Sicelo A. Mhlongo]

Fixes CVE-2023-4235
---
 src/smsutil.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/smsutil.c b/src/smsutil.c
index 8f578c22..bdb1d04f 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -1226,10 +1226,16 @@ static gboolean decode_deliver_report(const unsigned char *pdu, int len,
 			return FALSE;
 
 		if (out->type == SMS_TYPE_DELIVER_REPORT_ERROR) {
+			if (expected > (int) sizeof(out->deliver_err_report.ud))
+				return FALSE;
+
 			out->deliver_err_report.udl = udl;
 			memcpy(out->deliver_err_report.ud,
 					pdu + offset, expected);
 		} else {
+			if (expected > (int) sizeof(out->deliver_ack_report.ud))
+				return FALSE;
+
 			out->deliver_ack_report.udl = udl;
 			memcpy(out->deliver_ack_report.ud,
 					pdu + offset, expected);
-- 
2.45.2

[PATCH 2/2] smsutil: check status report fits in buffer

[Sicelo A. Mhlongo]

Fixes CVE-2023-4232
---
 src/smsutil.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/smsutil.c b/src/smsutil.c
index bdb1d04f..8c1aaad3 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -1077,6 +1077,9 @@ static gboolean decode_status_report(const unsigned char *pdu, int len,
 		if ((len - offset) < expected)
 			return FALSE;
 
+		if (expected > (int)sizeof(out->status_report.ud))
+			return FALSE;
+
 		memcpy(out->status_report.ud, pdu + offset, expected);
 	}
 
-- 
2.45.2

Re: [PATCH 2/2] smsutil: check status report fits in buffer

[Marcel Holtmann]

Hi Sicelo,

> Fixes CVE-2023-4232
> ---
> src/smsutil.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/src/smsutil.c b/src/smsutil.c
> index bdb1d04f..8c1aaad3 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -1077,6 +1077,9 @@ static gboolean decode_status_report(const unsigned char *pdu, int len,
> if ((len - offset) < expected)
> return FALSE;
> 
> + if (expected > (int)sizeof(out->status_report.ud))
> + return FALSE;
> +

every time we do casting, I would asked myself if the variable really has the right type and if casting could be avoided.

For example, what is the reason that sms_udl_in_bytes() returns int instead of size_t or unsigned int? Denis?

Regards

Marcel

Re: [PATCH 2/2] smsutil: check status report fits in buffer

[Denis Kenzior]

Hi Marcel,

On 12/4/24 3:55 AM, Marcel Holtmann wrote:
> Hi Sicelo,
> 
>> Fixes CVE-2023-4232
>> ---
>> src/smsutil.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/src/smsutil.c b/src/smsutil.c
>> index bdb1d04f..8c1aaad3 100644
>> --- a/src/smsutil.c
>> +++ b/src/smsutil.c
>> @@ -1077,6 +1077,9 @@ static gboolean decode_status_report(const unsigned char *pdu, int len,
>> if ((len - offset) < expected)
>> return FALSE;
>>
>> + if (expected > (int)sizeof(out->status_report.ud))
>> + return FALSE;
>> +
> 
> every time we do casting, I would asked myself if the variable really has the right type and if casting could be avoided.
> 
> For example, what is the reason that sms_udl_in_bytes() returns int instead of size_t or unsigned int? Denis?
> 

No good reason.  It should be returning a size_t.

Regards,
-Denis

Re: [PATCH 1/2] smsutil: check deliver reports fit in buffer

[patchwork-bot+ofono]

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Wed,  4 Dec 2024 10:18:51 +0200 you wrote:
> Fixes CVE-2023-4235
> ---
>  src/smsutil.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Here is the summary with links:
  - [1/2] smsutil: check deliver reports fit in buffer
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=02aa0f9bad3d
  - [2/2] smsutil: check status report fits in buffer
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=2ff2da7ac374

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html