osmocom
/
libosmocore
mirror of https://gerrit.osmocom.org/libosmocore
11
0
Fork 0
Code Issues Releases Wiki Activity

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
This commit is contained in:
Harald Welte 2020-12-03 15:53:59 +01:00 committed by laforge
parent 89106524a0
commit 2d9ce71fcb
2 changed files with 32 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
src/gb/gprs_bssgp.c
View File

@ -351,7 +351,7 @@ static int bssgp_rx_bvc_reset(struct msgb *msg, struct tlv_parsed *tp,
/* 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 @@ static int bssgp_rx_ul_ud(struct msgb *msg, struct tlv_parsed *tp,
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 @@ static int bssgp_rx_suspend(struct msgb *msg, struct tlv_parsed *tp)
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 @@ static int bssgp_rx_resume(struct msgb *msg, struct tlv_parsed *tp)
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 @@ static int bssgp_rx_llc_disc(struct msgb *msg, struct tlv_parsed *tp,
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 @@ int bssgp_rx_status(struct msgb *msg, struct tlv_parsed *tp,
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 @@ int bssgp_rx_status(struct msgb *msg, struct tlv_parsed *tp,
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 @@ static int bssgp_rx_fc_bvc(struct msgb *msg, struct tlv_parsed *tp,
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 @@ static int bssgp_rx_sign(struct msgb *msg, struct tlv_parsed *tp,
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 @@ static int bssgp_rx_sign(struct msgb *msg, struct tlv_parsed *tp,
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 @@ static int bssgp_rx_sign(struct msgb *msg, struct tlv_parsed *tp,
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 @@ int bssgp_rcvmsg(struct msgb *msg)
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 */

13
src/gb/gprs_bssgp_bss.c
View File

@ -511,24 +511,24 @@ int bssgp_rx_paging(struct bssgp_paging_info *pinfo,
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 @@ int bssgp_rx_paging(struct bssgp_paging_info *pinfo,
}
/* 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));