fixeria has uploaded this change for review.

View Change

sccp2sua: fix buffer overrun in osmo_isup_party_parse()

Add an out_digits_size parameter and check that the caller-provided
buffer is large enough to hold the generated digits plus the
terminating NUL, instead of blindly writing up to 2 * in_num_bytes
characters into it.

Change-Id: Iccfbcf22a719544399c7a524b293de9e1a040cf8
Fixes: OS#7038
---
M src/sccp2sua.c
M src/xua_internal.h
M tests/xua/xua_test.c
3 files changed, 17 insertions(+), 8 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/63/43163/1

diff --git a/src/sccp2sua.c b/src/sccp2sua.c
index 9ca1d7a..08f0776 100644
--- a/src/sccp2sua.c
+++ b/src/sccp2sua.c
@@ -57,17 +57,23 @@
 
 /*! \brief Parse ISUP style address of BCD digets
  *  \param[out] out_digits user-allocated buffer for ASCII digits
+ *  \param[in] out_digits_size size of the user-allocated output buffer
  *  \param[in] in BCD-encoded digits
  *  \param[in] in_num_bytes Size of \ref in in bytes
  *  \param[in] odd Odd (true) or even (false) number of digits
- *  \returns number of digits generated
+ *  \returns number of digits generated; negative on error
  * */
-int osmo_isup_party_parse(char *out_digits, const uint8_t *in,
-			    unsigned int in_num_bytes, bool odd)
+int osmo_isup_party_parse(char *out_digits, size_t out_digits_size,
+			  const uint8_t *in, unsigned int in_num_bytes, bool odd)
 {
 	char *out = out_digits;
 	unsigned int i;
 
+	/* The output buffer must be large enough to accomodate for
+	 * double the amount of bytes in the input plus the '\0' symbol. */
+	if (in_num_bytes * 2 >= out_digits_size)
+		return -E2BIG;
+
 	for (i = 0; i < in_num_bytes; i++) {
 		*out_digits++ = osmo_bcd2char(in[i] & 0x0F);
 		if (i+1 == in_num_bytes && odd)
@@ -203,7 +209,8 @@
 			sca->global_title_indicator);
 		return -EINVAL;
 	}
-	rc = osmo_isup_party_parse(out->gt.digits, cur, (addr+addrlen-cur), odd);
+	rc = osmo_isup_party_parse(out->gt.digits, sizeof(out->gt.digits),
+				   cur, (addr+addrlen-cur), odd);
 	if (rc < 0)
 		return rc;
 
diff --git a/src/xua_internal.h b/src/xua_internal.h
index 3857a72..5195b35 100644
--- a/src/xua_internal.h
+++ b/src/xua_internal.h
@@ -140,8 +140,8 @@
 int ipa_tx_xua_as(struct osmo_ss7_as *as, struct xua_msg *xua);
 int ipa_rx_msg(struct osmo_ss7_asp *asp, struct msgb *msg, uint8_t sls);
 
-int osmo_isup_party_parse(char *out_digits, const uint8_t *in,
-			unsigned int in_num_bytes, bool odd);
+int osmo_isup_party_parse(char *out_digits, size_t out_digits_size,
+			  const uint8_t *in, unsigned int in_num_bytes, bool odd);
 int osmo_sccp_addr_parse(struct osmo_sccp_addr *out,
 			const uint8_t *addr, unsigned int addrlen);
 int osmo_sccp_addr_encode(struct msgb *msg, const struct osmo_sccp_addr *in);
diff --git a/tests/xua/xua_test.c b/tests/xua/xua_test.c
index 0759ff7..3fc04a6 100644
--- a/tests/xua/xua_test.c
+++ b/tests/xua/xua_test.c
@@ -42,12 +42,14 @@
 	char digits[23] = "";
 	int rc;
 
-	rc = osmo_isup_party_parse(digits, party0, ARRAY_SIZE(party0), false);
+	rc = osmo_isup_party_parse(digits, sizeof(digits),
+				   party0, ARRAY_SIZE(party0), false);
 	printf("digits='%s' (%d)\n", digits, rc);
 	OSMO_ASSERT(rc == 8);
 	OSMO_ASSERT(!strcmp(digits, "01234567"));
 
-	rc = osmo_isup_party_parse(digits, party0, ARRAY_SIZE(party0), true);
+	rc = osmo_isup_party_parse(digits, sizeof(digits),
+				   party0, ARRAY_SIZE(party0), true);
 	printf("digits='%s' (%d)\n", digits, rc);
 	OSMO_ASSERT(rc == 7);
 	OSMO_ASSERT(!strcmp(digits, "0123456"));

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