diff options
author | Harald Welte <laforge@osmocom.org> | 2020-12-03 16:01:02 +0100 |
---|---|---|
committer | laforge <laforge@osmocom.org> | 2020-12-04 18:20:15 +0000 |
commit | 798efea27e5a736bd539c60a2e05ecaccfe986ba (patch) | |
tree | d771473ab63cee4d2a9b5c725ecbcbbccdbbe685 | |
parent | 2d9ce71fcbe3803adae6cfa85fd3e697f89428e9 (diff) |
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
-rw-r--r-- | src/gb/gprs_ns2.c | 8 | ||||
-rw-r--r-- | src/gb/gprs_ns2_message.c | 19 |
2 files changed, 14 insertions, 13 deletions
diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c index 93807f00..d90ba850 100644 --- a/src/gb/gprs_ns2.c +++ b/src/gb/gprs_ns2.c @@ -590,7 +590,7 @@ static int reject_status_msg(struct msgb *orig_msg, struct tlv_parsed *tp, struc 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 @@ static int reject_status_msg(struct msgb *orig_msg, struct tlv_parsed *tp, struc 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 @@ enum gprs_ns2_cs ns2_create_vc(struct gprs_ns2_vc_bind *bind, 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 69c833e2..eb9a1984 100644 --- a/src/gb/gprs_ns2_message.c +++ b/src/gb/gprs_ns2_message.c @@ -66,7 +66,8 @@ enum ns_ctr { 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(struct gprs_ns2_vc *nsvc, struct msgb *msg, s 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_reset_ack(struct gprs_ns2_vc *nsvc, struct msgb *ms 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(struct gprs_ns2_vc *nsvc, struct msgb *msg, s 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_block_ack(struct gprs_ns2_vc *nsvc, struct msgb *ms 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 @@ static int gprs_ns2_validate_status(struct gprs_ns2_vc *nsvc, struct msgb *msg, 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 @@ static int gprs_ns2_validate_status(struct gprs_ns2_vc *nsvc, struct msgb *msg, 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; } |