Change in ...libosmocore[master]: gsm48_decode_bcd_number2: fix ENOSPC edge case

[laforge]

laforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/libosmocore/+/14397 )

Change subject: gsm48_decode_bcd_number2: fix ENOSPC edge case
......................................................................

gsm48_decode_bcd_number2: fix ENOSPC edge case

Return ENOSPC if the decoding buffer is one byte too small, instead of
returning 0 and silently truncating the string. Add a new "truncated"
variable to detect if the loop breaks in the final iteration.

The string is not truncated if there is exactly one 0xf ('\0') higher
nibble remaining. This is covered by the existing test case "long
15-digit (maximum) MSISDN, limited buffer".

Related: OS#4049
Change-Id: Ie05900aca50cc7fe8a45d17844dbfcd905fd82fe
---
M src/gsm/gsm48_ie.c
M tests/gsm0408/gsm0408_test.c
M tests/gsm0408/gsm0408_test.ok
3 files changed, 28 insertions(+), 3 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/src/gsm/gsm48_ie.c b/src/gsm/gsm48_ie.c
index 59f931b..31028ba 100644
--- a/src/gsm/gsm48_ie.c
+++ b/src/gsm/gsm48_ie.c
@@ -82,6 +82,7 @@
 {
 	uint8_t in_len;
 	int i;
+	bool truncated = false;
 	if (output_len < 1)
 		return -ENOSPC;
 	*output = '\0';
@@ -94,14 +95,23 @@
 
 	for (i = 1 + h_len; i <= in_len; i++) {
 		/* lower nibble */
-		if (output_len <= 1)
+		if (output_len <= 1) {
+			truncated = true;
 			break;
+		}
 		*output++ = bcd_num_digits[bcd_lv[i] & 0xf];
 		output_len--;
 
 		/* higher nibble */
-		if (output_len <= 1)
+		if (output_len <= 1) {
+			/* not truncated if there is exactly one 0xf ('\0') higher nibble remaining */
+			if (i == in_len && (bcd_lv[i] & 0xf0) == 0xf0) {
+				break;
+			}
+
+			truncated = true;
 			break;
+		}
 		*output++ = bcd_num_digits[bcd_lv[i] >> 4];
 		output_len--;
 	}
@@ -109,7 +119,7 @@
 		*output++ = '\0';
 
 	/* Indicate whether the output was truncated */
-	if (i < in_len)
+	if (truncated)
 		return -ENOSPC;
 
 	return 0;
diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c
index b5f8061..db1d45a 100644
--- a/tests/gsm0408/gsm0408_test.c
+++ b/tests/gsm0408/gsm0408_test.c
@@ -727,6 +727,17 @@
 		.dec_ascii = "(none)",
 		.dec_rc = -EIO,
 	},
+	{
+		.test_name = "decoding buffer is one byte too small (OS#4049)",
+
+		/* Decoding test */
+		.dec_hex   = "022143", /* "1234" */
+		.dec_ascii = "123",    /* '4' was truncated */
+		.dec_rc    = -ENOSPC,
+
+		/* Buffer length limitations */
+		.dec_buf_lim = 4,
+	},
 };
 
 static void test_bcd_number_encode_decode()
diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok
index 844c201..d343869 100644
--- a/tests/gsm0408/gsm0408_test.ok
+++ b/tests/gsm0408/gsm0408_test.ok
@@ -186,6 +186,10 @@
   - Decoding HEX (buffer limit=0) ''...
     - Expected: (rc=-5) '(none)'
     -   Actual: (rc=-5) '(none)'
+- Running test: decoding buffer is one byte too small (OS#4049)
+  - Decoding HEX (buffer limit=4) '022143'...
+    - Expected: (rc=-28) '123'
+    -   Actual: (rc=-28) '123'
 
 Constructed RA:
 077-121-666-5

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/14397
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ie05900aca50cc7fe8a45d17844dbfcd905fd82fe
Gerrit-Change-Number: 14397
Gerrit-PatchSet: 4
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190609/55fe8a30/attachment.htm>