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: adam.bedard@gmail.com
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;