laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/43229?usp=email )

Change subject: sgs_iface: guard against over-long MME name IEs.
......................................................................

sgs_iface: guard against over-long MME name IEs.

The function decode_mme_name decodes the MME name from the given
tlv_parsed struct a buffer referenced by mme_name. Since the maximum
length of the MME name is fixed, the length of the output buffer is
also fixed (SGS_MME_NAME_LEN bytes + 1 byte string terminator).

Unfortunately the function does not guard against over-long input.
When the SGSAP_IE_MME_NAME IE is longer than SGS_MME_NAME_LEN, then
either memcpy or osmo_apn_to_str may overflow the output buffer.

Bug reported by: [email protected]

Change-Id: I9e845ad1568cb3a88c90f81655c30cac862f83ec
Related: OS#7058
---
M src/libmsc/sgs_iface.c
1 file changed, 7 insertions(+), 2 deletions(-)

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




diff --git a/src/libmsc/sgs_iface.c b/src/libmsc/sgs_iface.c
index 178aa64..4425a93 100644
--- a/src/libmsc/sgs_iface.c
+++ b/src/libmsc/sgs_iface.c
@@ -160,7 +160,7 @@
 }

 /* Decode and verify MME name */
-static int decode_mme_name(char *mme_name, const struct tlv_parsed *tp)
+static int decode_mme_name(char *mme_name, size_t mme_name_len, const struct 
tlv_parsed *tp)
 {
        const uint8_t *mme_name_enc = TLVP_VAL_MINLEN(tp, SGSAP_IE_MME_NAME, 
SGS_MME_NAME_LEN);
        struct osmo_gummei gummei;
@@ -168,6 +168,11 @@
        if (!mme_name_enc)
                return -EINVAL;

+       /* do not accept over-long SGSAP_IE_MME_NAME IEs which would exceed the 
length
+        * of the output buffer. */
+       if (TLVP_LEN(tp, SGSAP_IE_MME_NAME) >= mme_name_len)
+               return -EINVAL;
+
        /* some implementations use FDQN format violating TS 29.118 9.3.14 */
        if (!osmo_parse_mme_domain(&gummei, (const char *) mme_name_enc)) {
                memcpy(mme_name, mme_name_enc, TLVP_LEN(tp, SGSAP_IE_MME_NAME));
@@ -1040,7 +1045,7 @@
        }

        if (TLVP_PRESENT(&tp, SGSAP_IE_MME_NAME)) {
-               if (decode_mme_name(mme_name, &tp) != 0) {
+               if (decode_mme_name(mme_name, sizeof(mme_name), &tp) != 0) {
                        TX_STATUS_AND_LOG(sgc, msg_type, 
SGSAP_SGS_CAUSE_INVALID_MAND_IE,
                                          "SGsAP Message %s with invalid 
MME-Name, dropping\n");
                        goto error;

--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/43229?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9e845ad1568cb3a88c90f81655c30cac862f83ec
Gerrit-Change-Number: 43229
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>

Reply via email to