This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
laforge gerrit-no-reply at lists.osmocom.orglaforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21494 ) Change subject: gprs_ns2: Use TLVP_PRES_LEN instead of TLVP_PRESENT ...................................................................... gprs_ns2: Use TLVP_PRES_LEN instead of TLVP_PRESENT With TLVP_PRESENT we only check if a given TLV/IE is present, but don't verify that it's length matches our expectation. This can lead to out-of-bounds reads, so let's always use TLVP_PRES_LEN. Change-Id: I4c438bc82ea6a48243db568f96a234adf784dc0b --- M src/gb/gprs_ns2.c M src/gb/gprs_ns2_message.c 2 files changed, 14 insertions(+), 13 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c index 93807f0..d90ba85 100644 --- a/src/gb/gprs_ns2.c +++ b/src/gb/gprs_ns2.c @@ -590,7 +590,7 @@ if (!msg) return -ENOMEM; - if (TLVP_PRESENT(tp, NS_IE_NSEI)) { + if (TLVP_PRES_LEN(tp, NS_IE_NSEI, 2)) { nsei = tlvp_val16be(tp, NS_IE_NSEI); LOGP(DLNS, LOGL_NOTICE, "NSEI=%u Rejecting message without NSVCI. Tx NS STATUS (cause=%s)\n", @@ -602,7 +602,7 @@ nsh->pdu_type = NS_PDUT_STATUS; msgb_tvlv_put(msg, NS_IE_CAUSE, 1, &_cause); - have_vci = TLVP_PRESENT(tp, NS_IE_VCI); + have_vci = TLVP_PRES_LEN(tp, NS_IE_VCI, 2); /* Section 9.2.7.1: Static conditions for NS-VCI */ if (cause == NS_CAUSE_NSVC_BLOCKED || @@ -822,8 +822,8 @@ return GPRS_NS2_CS_REJECTED; } - if (!TLVP_PRESENT(&tp, NS_IE_CAUSE) || - !TLVP_PRESENT(&tp, NS_IE_VCI) || !TLVP_PRESENT(&tp, NS_IE_NSEI)) { + if (!TLVP_PRES_LEN(&tp, NS_IE_CAUSE, 1) || + !TLVP_PRES_LEN(&tp, NS_IE_VCI, 2) || !TLVP_PRES_LEN(&tp, NS_IE_NSEI, 2)) { LOGP(DLNS, LOGL_ERROR, "NS RESET Missing mandatory IE\n"); rc = reject_status_msg(msg, &tp, reject, NS_CAUSE_MISSING_ESSENT_IE); return GPRS_NS2_CS_REJECTED; diff --git a/src/gb/gprs_ns2_message.c b/src/gb/gprs_ns2_message.c index 69c833e..eb9a198 100644 --- a/src/gb/gprs_ns2_message.c +++ b/src/gb/gprs_ns2_message.c @@ -66,7 +66,8 @@ static int gprs_ns2_validate_reset(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause) { - if (!TLVP_PRESENT(tp, NS_IE_CAUSE) || !TLVP_PRESENT(tp, NS_IE_VCI) || !TLVP_PRESENT(tp, NS_IE_NSEI)) { + if (!TLVP_PRES_LEN(tp, NS_IE_CAUSE, 1) || + !TLVP_PRES_LEN(tp, NS_IE_VCI, 2) || !TLVP_PRES_LEN(tp, NS_IE_NSEI, 2)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -76,7 +77,7 @@ static int gprs_ns2_validate_reset_ack(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause) { - if (!TLVP_PRESENT(tp, NS_IE_VCI) || !TLVP_PRESENT(tp, NS_IE_NSEI)) { + if (!TLVP_PRES_LEN(tp, NS_IE_VCI, 2) || !TLVP_PRES_LEN(tp, NS_IE_NSEI, 2)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -86,7 +87,7 @@ static int gprs_ns2_validate_block(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause) { - if (!TLVP_PRESENT(tp, NS_IE_VCI) || !TLVP_PRESENT(tp, NS_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, NS_IE_VCI, 2) || !TLVP_PRES_LEN(tp, NS_IE_CAUSE, 1)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -96,7 +97,7 @@ static int gprs_ns2_validate_block_ack(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause) { - if (!TLVP_PRESENT(tp, NS_IE_VCI)) { + if (!TLVP_PRES_LEN(tp, NS_IE_VCI, 2)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -107,7 +108,7 @@ static int gprs_ns2_validate_status(struct gprs_ns2_vc *nsvc, struct msgb *msg, struct tlv_parsed *tp, uint8_t *cause) { - if (!TLVP_PRESENT(tp, NS_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, NS_IE_CAUSE, 1)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -117,7 +118,7 @@ switch (_cause) { case NS_CAUSE_NSVC_BLOCKED: case NS_CAUSE_NSVC_UNKNOWN: - if (!TLVP_PRESENT(tp, NS_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, NS_IE_CAUSE, 1)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } @@ -127,19 +128,19 @@ case NS_CAUSE_PROTO_ERR_UNSPEC: case NS_CAUSE_INVAL_ESSENT_IE: case NS_CAUSE_MISSING_ESSENT_IE: - if (!TLVP_PRESENT(tp, NS_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, NS_IE_CAUSE, 1)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } break; case NS_CAUSE_BVCI_UNKNOWN: - if (!TLVP_PRESENT(tp, NS_IE_BVCI)) { + if (!TLVP_PRES_LEN(tp, NS_IE_BVCI, 2)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } break; case NS_CAUSE_UNKN_IP_TEST_FAILED: - if (!TLVP_PRESENT (tp, NS_IE_IPv4_LIST) && !TLVP_PRESENT(tp, NS_IE_IPv6_LIST)) { + if (!TLVP_PRESENT(tp, NS_IE_IPv4_LIST) && !TLVP_PRESENT(tp, NS_IE_IPv6_LIST)) { *cause = NS_CAUSE_MISSING_ESSENT_IE; return -1; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21494 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4c438bc82ea6a48243db568f96a234adf784dc0b Gerrit-Change-Number: 21494 Gerrit-PatchSet: 5 Gerrit-Owner: laforge <laforge at osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann at sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-CC: pespin <pespin at sysmocom.de> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201204/ba268ea7/attachment.htm>