dexter has uploaded this change for review.

View Change

gprs_sndcp_xid: double check buffer size after writing SAPIs

The function encode_pcomp_applicable_sapis writes the applicable
SAPIs to the output buffer. For this it requires 2 bytes. After
that, each encoder funtion writes the remaining information to
the buffer. The check at the beginning of each encoder function
is sufficient to ensure that the buffer is large enough.

However, this remains only true in case the number of bytes
that encode_pcomp_applicable_sapis writes is 2. If there are
changes to the spec or similar, the function might write more
than 2 bytes. So let's place an additional length check to be
on the secure side.

Change-Id: I059c903885e21215ad1e6b835f34db2c5f70d87a
Related: OS#7095
---
M src/sgsn/gprs_sndcp_xid.c
1 file changed, 20 insertions(+), 0 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/68/43968/1
diff --git a/src/sgsn/gprs_sndcp_xid.c b/src/sgsn/gprs_sndcp_xid.c
index 21b6030..67deaa9 100644
--- a/src/sgsn/gprs_sndcp_xid.c
+++ b/src/sgsn/gprs_sndcp_xid.c
@@ -115,6 +115,10 @@
dst += rc;
dst_counter += rc;

+ /* We still need to write 1 byte from here */
+ if (dst_maxlen - dst_counter < 1)
+ return -ENOMEM;
+
/* Encode s01 (see also: 3GPP TS 44.065, 6.5.2.1, Table 5) */
if (params->s01 < 0 || params->s01 > 255)
return -EINVAL;
@@ -154,6 +158,10 @@
dst += rc;
dst_counter += rc;

+ /* We still need to write 7 bytes from here */
+ if (dst_maxlen - dst_counter < 7)
+ return -ENOMEM;
+
/* Encode F_MAX_PERIOD (see also: 3GPP TS 44.065, 6.5.3.1, Table 6) */
if (params->f_max_period < 1 || params->f_max_period > 65535)
return -EINVAL;
@@ -233,6 +241,10 @@
dst += rc;
dst_counter += rc;

+ /* We still need to write 36 bytes from here */
+ if (dst_maxlen - dst_counter < 36)
+ return -ENOMEM;
+
/* Encode MAX_CID (see also: 3GPP TS 44.065, 6.5.4.1, Table 10) */
if (params->max_cid < 0 || params->max_cid > 16383)
return -EINVAL;
@@ -290,6 +302,10 @@
dst += rc;
dst_counter += rc;

+ /* We still need to write 4 bytes from here */
+ if (dst_maxlen - dst_counter < 4)
+ return -ENOMEM;
+
/* Encode P0 (see also: 3GPP TS 44.065, 6.6.2.1, Table 7a) */
if (params->p0 < 0 || params->p0 > 3)
return -EINVAL;
@@ -343,6 +359,10 @@
dst += rc;
dst_counter += rc;

+ /* We still need to write 10 bytes from here */
+ if (dst_maxlen - dst_counter < 10)
+ return -ENOMEM;
+
/* Encode C0 (see also: 3GPP TS 44.065, 6.6.3.1, Table 7c) */
if (params->c0 != 0x80 && params->c0 != 0xC0)
return -EINVAL;

To view, visit change 43968. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newchange
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I059c903885e21215ad1e6b835f34db2c5f70d87a
Gerrit-Change-Number: 43968
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier@sysmocom.de>