laforge has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43163?usp=email )
Change subject: sccp2sua: fix buffer overrun in osmo_isup_party_parse()
......................................................................
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
M tests/xua/xua_test.ok
4 files changed, 37 insertions(+), 8 deletions(-)
Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/src/sccp2sua.c b/src/sccp2sua.c
index 9ca1d7a..b8030a6 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 accommodate for
+ * the generated digits plus the '\0' symbol. */
+ if (in_num_bytes * 2 + (odd ? 0 : 1) > 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..3c5f5fd 100644
--- a/tests/xua/xua_test.c
+++ b/tests/xua/xua_test.c
@@ -42,15 +42,35 @@
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"));
+
+ /* out_digits_size exactly matches what's needed for an odd
+ * number of digits: must be accepted, not rejected with -E2BIG. */
+ rc = osmo_isup_party_parse(digits, 8, party0, ARRAY_SIZE(party0), true);
+ printf("digits='%s' (%d)\n", digits, rc);
+ OSMO_ASSERT(rc == 7);
+ OSMO_ASSERT(!strcmp(digits, "0123456"));
+
+ /* one byte too small: must be rejected. */
+ rc = osmo_isup_party_parse(digits, 7, party0, ARRAY_SIZE(party0), true);
+ printf("rc=%d\n", rc);
+ OSMO_ASSERT(rc == -E2BIG);
+
+ /* even number of digits, exact fit and one byte too small. */
+ rc = osmo_isup_party_parse(digits, 9, party0, ARRAY_SIZE(party0),
false);
+ OSMO_ASSERT(rc == 8);
+ rc = osmo_isup_party_parse(digits, 8, party0, ARRAY_SIZE(party0),
false);
+ OSMO_ASSERT(rc == -E2BIG);
}
/* SCCP Address Parsing */
diff --git a/tests/xua/xua_test.ok b/tests/xua/xua_test.ok
index 9d44775..02e5f49 100644
--- a/tests/xua/xua_test.ok
+++ b/tests/xua/xua_test.ok
@@ -1,5 +1,7 @@
digits='01234567' (8)
digits='0123456' (7)
+digits='0123456' (7)
+rc=-7
sccp_addr_parse test case 0
expected: RI=1,SSN=6,GTI=4,GT=(TT=0,NPL=1,NAI=4,DIG=919969679389)
parsed: RI=1,SSN=6,GTI=4,GT=(TT=0,NPL=1,NAI=4,DIG=919969679389)
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/43163?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: Iccfbcf22a719544399c7a524b293de9e1a040cf8
Gerrit-Change-Number: 43163
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
