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
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));