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/+/21493 ) Change subject: bssgp: Use TLVP_PRES_LEN instead of TLVP_PRESENT ...................................................................... bssgp: 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: I56e8b31ce51602d2681e3db501c48f84bfe7e438 --- M src/gb/gprs_bssgp.c M src/gb/gprs_bssgp_bss.c 2 files changed, 32 insertions(+), 33 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c index ebbfab1..7fb3a30 100644 --- a/src/gb/gprs_bssgp.c +++ b/src/gb/gprs_bssgp.c @@ -351,7 +351,7 @@ /* When we receive a BVC-RESET PDU (at least of a PTP BVCI), the BSS * informs us about its RAC + Cell ID, so we can create a mapping */ if (bvci != 0 && bvci != 1) { - if (!TLVP_PRESENT(tp, BSSGP_IE_CELL_ID)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_CELL_ID, 8)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx RESET " "missing mandatory IE\n", bvci); return -EINVAL; @@ -467,7 +467,7 @@ DEBUGP(DBSSGP, "BSSGP TLLI=0x%08x Rx UPLINK-UNITDATA\n", msgb_tlli(msg)); /* Cell ID and LLC_PDU are the only mandatory IE */ - if (!TLVP_PRESENT(tp, BSSGP_IE_CELL_ID) || + if (!TLVP_PRES_LEN(tp, BSSGP_IE_CELL_ID, 8) || !TLVP_PRESENT(tp, BSSGP_IE_LLC_PDU)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP TLLI=0x%08x Rx UL-UD " "missing mandatory IE\n", msgb_tlli(msg)); @@ -497,8 +497,8 @@ uint16_t ns_bvci = msgb_bvci(msg), nsei = msgb_nsei(msg); int rc; - if (!TLVP_PRESENT(tp, BSSGP_IE_TLLI) || - !TLVP_PRESENT(tp, BSSGP_IE_ROUTEING_AREA)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_TLLI, 4) || + !TLVP_PRES_LEN(tp, BSSGP_IE_ROUTEING_AREA, 6)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx SUSPEND " "missing mandatory IE\n", ns_bvci); return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE, NULL, msg); @@ -538,9 +538,9 @@ uint16_t ns_bvci = msgb_bvci(msg), nsei = msgb_nsei(msg); int rc; - if (!TLVP_PRESENT(tp, BSSGP_IE_TLLI) || - !TLVP_PRESENT(tp, BSSGP_IE_ROUTEING_AREA) || - !TLVP_PRESENT(tp, BSSGP_IE_SUSPEND_REF_NR)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_TLLI, 4 ) || + !TLVP_PRES_LEN(tp, BSSGP_IE_ROUTEING_AREA, 6) || + !TLVP_PRES_LEN(tp, BSSGP_IE_SUSPEND_REF_NR, 1)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx RESUME " "missing mandatory IE\n", ns_bvci); return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE, NULL, msg); @@ -580,16 +580,16 @@ uint32_t tlli = 0; uint16_t nsei = msgb_nsei(msg); - if (!TLVP_PRESENT(tp, BSSGP_IE_TLLI) || - !TLVP_PRESENT(tp, BSSGP_IE_LLC_FRAMES_DISCARDED) || - !TLVP_PRESENT(tp, BSSGP_IE_BVCI) || - !TLVP_PRESENT(tp, BSSGP_IE_NUM_OCT_AFF)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_TLLI, 4) || + !TLVP_PRES_LEN(tp, BSSGP_IE_LLC_FRAMES_DISCARDED, 1) || + !TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_NUM_OCT_AFF, 3)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx LLC DISCARDED " "missing mandatory IE\n", ctx->bvci); + return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE, NULL, msg); } - if (TLVP_PRESENT(tp, BSSGP_IE_TLLI)) - tlli = tlvp_val32be(tp, BSSGP_IE_TLLI); + tlli = tlvp_val32be(tp, BSSGP_IE_TLLI); DEBUGP(DBSSGP, "BSSGP BVCI=%u TLLI=%08x Rx LLC DISCARDED\n", ctx->bvci, tlli); @@ -615,7 +615,7 @@ struct osmo_bssgp_prim nmp; enum gprs_bssgp_cause cause; - if (!TLVP_PRESENT(tp, BSSGP_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_CAUSE, 1)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx STATUS " "missing mandatory IE\n", bvci); cause = BSSGP_CAUSE_PROTO_ERR_UNSPEC; @@ -627,7 +627,7 @@ bvci, bssgp_cause_str(cause)); if (cause == BSSGP_CAUSE_BVCI_BLOCKED || cause == BSSGP_CAUSE_UNKNOWN_BVCI) { - if (!TLVP_PRESENT(tp, BSSGP_IE_BVCI)) + if (!TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2)) LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx STATUS cause=%s " "missing conditional BVCI IE\n", @@ -882,11 +882,11 @@ DEBUGP(DBSSGP, "BSSGP BVCI=%u Rx Flow Control BVC\n", bctx->bvci); - if (!TLVP_PRESENT(tp, BSSGP_IE_TAG) || - !TLVP_PRESENT(tp, BSSGP_IE_BVC_BUCKET_SIZE) || - !TLVP_PRESENT(tp, BSSGP_IE_BUCKET_LEAK_RATE) || - !TLVP_PRESENT(tp, BSSGP_IE_BMAX_DEFAULT_MS) || - !TLVP_PRESENT(tp, BSSGP_IE_R_DEFAULT_MS)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_TAG, 1) || + !TLVP_PRES_LEN(tp, BSSGP_IE_BVC_BUCKET_SIZE, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_BUCKET_LEAK_RATE, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_BMAX_DEFAULT_MS, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_R_DEFAULT_MS,2)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP BVCI=%u Rx FC BVC " "missing mandatory IE\n", bctx->bvci); return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE, NULL, msg); @@ -1040,8 +1040,8 @@ break; case BSSGP_PDUT_BVC_BLOCK: /* BSS tells us that BVC shall be blocked */ - if (!TLVP_PRESENT(tp, BSSGP_IE_BVCI) || - !TLVP_PRESENT(tp, BSSGP_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_CAUSE, 1)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP Rx BVC-BLOCK " "missing mandatory IE\n"); goto err_mand_ie; @@ -1050,7 +1050,7 @@ break; case BSSGP_PDUT_BVC_UNBLOCK: /* BSS tells us that BVC shall be unblocked */ - if (!TLVP_PRESENT(tp, BSSGP_IE_BVCI)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP Rx BVC-UNBLOCK " "missing mandatory IE\n"); goto err_mand_ie; @@ -1062,8 +1062,8 @@ break; case BSSGP_PDUT_BVC_RESET: /* BSS tells us that BVC init is required */ - if (!TLVP_PRESENT(tp, BSSGP_IE_BVCI) || - !TLVP_PRESENT(tp, BSSGP_IE_CAUSE)) { + if (!TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2) || + !TLVP_PRES_LEN(tp, BSSGP_IE_CAUSE, 1)) { LOGP(DBSSGP, LOGL_ERROR, "BSSGP Rx BVC-RESET " "missing mandatory IE\n"); goto err_mand_ie; @@ -1135,7 +1135,7 @@ return rc; } - if (bvci == BVCI_SIGNALLING && TLVP_PRESENT(&tp, BSSGP_IE_BVCI)) + if (bvci == BVCI_SIGNALLING && TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2)) bvci = tlvp_val16be(&tp, BSSGP_IE_BVCI); /* look-up or create the BTS context for this BVC */ diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c index 59b06f0..462666a 100644 --- a/src/gb/gprs_bssgp_bss.c +++ b/src/gb/gprs_bssgp_bss.c @@ -511,24 +511,24 @@ TLVP_LEN(&tp, BSSGP_IE_IMSI)); /* DRX Parameters */ - if (!TLVP_PRESENT(&tp, BSSGP_IE_DRX_PARAMS)) + if (!TLVP_PRES_LEN(&tp, BSSGP_IE_DRX_PARAMS, 2)) goto err_mand_ie; pinfo->drx_params = tlvp_val16be(&tp, BSSGP_IE_DRX_PARAMS); /* Scope */ - if (TLVP_PRESENT(&tp, BSSGP_IE_BSS_AREA_ID)) { + if (TLVP_PRES_LEN(&tp, BSSGP_IE_BSS_AREA_ID, 1)) { pinfo->scope = BSSGP_PAGING_BSS_AREA; - } else if (TLVP_PRESENT(&tp, BSSGP_IE_LOCATION_AREA)) { + } else if (TLVP_PRES_LEN(&tp, BSSGP_IE_LOCATION_AREA, 5)) { pinfo->scope = BSSGP_PAGING_LOCATION_AREA; memcpy(ra, TLVP_VAL(&tp, BSSGP_IE_LOCATION_AREA), TLVP_LEN(&tp, BSSGP_IE_LOCATION_AREA)); gsm48_parse_ra(&pinfo->raid, ra); - } else if (TLVP_PRESENT(&tp, BSSGP_IE_ROUTEING_AREA)) { + } else if (TLVP_PRES_LEN(&tp, BSSGP_IE_ROUTEING_AREA, 6)) { pinfo->scope = BSSGP_PAGING_ROUTEING_AREA; memcpy(ra, TLVP_VAL(&tp, BSSGP_IE_ROUTEING_AREA), TLVP_LEN(&tp, BSSGP_IE_ROUTEING_AREA)); gsm48_parse_ra(&pinfo->raid, ra); - } else if (TLVP_PRESENT(&tp, BSSGP_IE_BVCI)) { + } else if (TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2)) { pinfo->scope = BSSGP_PAGING_BVCI; pinfo->bvci = tlvp_val16be(&tp, BSSGP_IE_BVCI); } else @@ -546,8 +546,7 @@ } /* Optional (P-)TMSI */ - if (TLVP_PRESENT(&tp, BSSGP_IE_TMSI) && - TLVP_LEN(&tp, BSSGP_IE_TMSI) >= 4) { + if (TLVP_PRES_LEN(&tp, BSSGP_IE_TMSI, 4)) { if (!pinfo->ptmsi) pinfo->ptmsi = talloc_zero_size(pinfo, sizeof(uint32_t)); *(pinfo->ptmsi) = osmo_load32be(TLVP_VAL(&tp, BSSGP_IE_TMSI)); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21493 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I56e8b31ce51602d2681e3db501c48f84bfe7e438 Gerrit-Change-Number: 21493 Gerrit-PatchSet: 6 Gerrit-Owner: laforge <laforge at osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann at sysmocom.de> Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: 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/ec9aa913/attachment.htm>