aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorHarald Welte <laforge@osmocom.org>2020-12-03 15:36:59 +0100
committerHarald Welte <laforge@osmocom.org>2020-12-05 11:44:39 +0100
commit6c4c6f08ae13d89b375584988ed1f0556417a7cf (patch)
tree51fb5104d40ea410272d5715c95820b9f8b82949 /src
parentfb7f8c5f0771ad65b57cfce4357805be8c673ae7 (diff)
gb_proxy: Use TLVP_PRES_LEN instead of TLVP_PRESENT
With TLVP_PRESENT we only check if a tiven 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: I1519cff0f6b2fe77f9a91eee17e0055d9df1bce6
Diffstat (limited to 'src')
-rw-r--r--src/gbproxy/gb_proxy.c28
-rw-r--r--src/gbproxy/gb_proxy_peer.c6
2 files changed, 17 insertions, 17 deletions
diff --git a/src/gbproxy/gb_proxy.c b/src/gbproxy/gb_proxy.c
index 8b103c8a..c1304667 100644
--- a/src/gbproxy/gb_proxy.c
+++ b/src/gbproxy/gb_proxy.c
@@ -1017,7 +1017,7 @@ static int gbprox_rx_bvc_reset_from_bss(struct gbproxy_config *cfg, struct msgb
struct gbproxy_peer *from_peer = NULL;
uint16_t bvci;
- 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)) {
rate_ctr_inc(&cfg->ctrg->ctr[GBPROX_GLOB_CTR_PROTO_ERR_BSS]);
return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE, NULL, msg);
}
@@ -1076,7 +1076,7 @@ static int gbprox_rx_bvc_reset_from_bss(struct gbproxy_config *cfg, struct msgb
gbproxy_peer_move(from_peer, nse_new);
}
- if (TLVP_PRESENT(tp, BSSGP_IE_CELL_ID)) {
+ if (TLVP_PRES_LEN(tp, BSSGP_IE_CELL_ID, 8)) {
struct gprs_ra_id raid;
/* We have a Cell Identifier present in this
* PDU, this means we can extend our local
@@ -1132,7 +1132,7 @@ static int gbprox_rx_sig_from_bss(struct gbproxy_config *cfg,
* area identification. The snooped information is then used
* for routing the {SUSPEND,RESUME}_[N]ACK back to the correct
* BSSGP */
- if (!TLVP_PRESENT(&tp, BSSGP_IE_ROUTEING_AREA))
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_ROUTEING_AREA, 6))
goto err_mand_ie;
from_peer = gbproxy_peer_by_nsei(cfg, nsei);
if (!from_peer)
@@ -1188,7 +1188,7 @@ static int gbprox_rx_paging(struct gbproxy_config *cfg, struct msgb *msg, struct
LOGP(DGPRS, LOGL_INFO, "NSE(%05u/SGSN) BSSGP PAGING\n",
nsei);
- if (TLVP_PRESENT(tp, BSSGP_IE_BVCI)) {
+ if (TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2)) {
uint16_t bvci = ntohs(tlvp_val16_unal(tp, BSSGP_IE_BVCI));
errctr = GBPROX_GLOB_CTR_OTHER_ERR;
peer = gbproxy_peer_by_bvci(cfg, bvci);
@@ -1200,7 +1200,7 @@ static int gbprox_rx_paging(struct gbproxy_config *cfg, struct msgb *msg, struct
}
LOGPBVC(peer, LOGL_INFO, "routing by BVCI\n");
return gbprox_relay2peer(msg, peer, ns_bvci);
- } else if (TLVP_PRESENT(tp, BSSGP_IE_ROUTEING_AREA)) {
+ } else if (TLVP_PRES_LEN(tp, BSSGP_IE_ROUTEING_AREA, 6)) {
errctr = GBPROX_GLOB_CTR_INV_RAI;
/* iterate over all peers and dispatch the paging to each matching one */
llist_for_each_entry(nse, &cfg->nse_peers, list) {
@@ -1214,7 +1214,7 @@ static int gbprox_rx_paging(struct gbproxy_config *cfg, struct msgb *msg, struct
}
}
}
- } else if (TLVP_PRESENT(tp, BSSGP_IE_LOCATION_AREA)) {
+ } else if (TLVP_PRES_LEN(tp, BSSGP_IE_LOCATION_AREA, 5)) {
errctr = GBPROX_GLOB_CTR_INV_LAI;
/* iterate over all peers and dispatch the paging to each matching one */
llist_for_each_entry(nse, &cfg->nse_peers, list) {
@@ -1228,7 +1228,7 @@ static int gbprox_rx_paging(struct gbproxy_config *cfg, struct msgb *msg, struct
}
}
}
- } else if (TLVP_PRESENT(tp, BSSGP_IE_BSS_AREA_ID)) {
+ } else if (TLVP_PRES_LEN(tp, BSSGP_IE_BSS_AREA_ID, 1)) {
/* iterate over all peers and dispatch the paging to each matching one */
llist_for_each_entry(nse, &cfg->nse_peers, list) {
llist_for_each_entry(peer, &nse->bts_peers, list) {
@@ -1264,7 +1264,7 @@ static int rx_reset_from_sgsn(struct gbproxy_config *cfg,
struct gbproxy_peer *peer;
uint16_t ptp_bvci;
- if (!TLVP_PRESENT(tp, BSSGP_IE_BVCI)) {
+ if (!TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2)) {
rate_ctr_inc(&cfg->ctrg->
ctr[GBPROX_GLOB_CTR_PROTO_ERR_SGSN]);
return bssgp_tx_status(BSSGP_CAUSE_MISSING_MAND_IE,
@@ -1347,7 +1347,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
if (cfg->route_to_sgsn2 && nsei == cfg->nsip_sgsn2_nsei)
break;
/* simple case: BVCI IE is mandatory */
- if (!TLVP_PRESENT(&tp, BSSGP_IE_BVCI))
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2))
goto err_mand_ie;
bvci = ntohs(tlvp_val16_unal(&tp, BSSGP_IE_BVCI));
if (bvci == BVCI_SIGNALLING) {
@@ -1358,7 +1358,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
break;
case BSSGP_PDUT_FLUSH_LL:
/* simple case: BVCI IE is mandatory */
- if (!TLVP_PRESENT(&tp, BSSGP_IE_BVCI))
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2))
goto err_mand_ie;
bvci = ntohs(tlvp_val16_unal(&tp, BSSGP_IE_BVCI));
rc = gbprox_relay2bvci(cfg, msg, bvci, ns_bvci);
@@ -1372,7 +1372,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
/* Some exception has occurred */
LOGP(DGPRS, LOGL_NOTICE,
"NSE(%05u/SGSN) BSSGP STATUS ", nsei);
- if (!TLVP_PRESENT(&tp, BSSGP_IE_CAUSE)) {
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_CAUSE, 1)) {
LOGPC(DGPRS, LOGL_NOTICE, "\n");
goto err_mand_ie;
}
@@ -1380,7 +1380,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
LOGPC(DGPRS, LOGL_NOTICE,
"cause=0x%02x(%s) ", *TLVP_VAL(&tp, BSSGP_IE_CAUSE),
bssgp_cause_str(cause));
- if (TLVP_PRESENT(&tp, BSSGP_IE_BVCI)) {
+ if (TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2)) {
bvci = ntohs(tlvp_val16_unal(&tp, BSSGP_IE_BVCI));
LOGPC(DGPRS, LOGL_NOTICE, "BVCI=%05u\n", bvci);
@@ -1395,7 +1395,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
case BSSGP_PDUT_RESUME_ACK:
case BSSGP_PDUT_RESUME_NACK:
/* RAI IE is mandatory */
- if (!TLVP_PRESENT(&tp, BSSGP_IE_ROUTEING_AREA))
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_ROUTEING_AREA, 6))
goto err_mand_ie;
peer = gbproxy_peer_by_rai(cfg, TLVP_VAL(&tp, BSSGP_IE_ROUTEING_AREA));
if (!peer)
@@ -1404,7 +1404,7 @@ static int gbprox_rx_sig_from_sgsn(struct gbproxy_config *cfg,
break;
case BSSGP_PDUT_BVC_BLOCK_ACK:
case BSSGP_PDUT_BVC_UNBLOCK_ACK:
- if (!TLVP_PRESENT(&tp, BSSGP_IE_BVCI))
+ if (!TLVP_PRES_LEN(&tp, BSSGP_IE_BVCI, 2))
goto err_mand_ie;
bvci = ntohs(tlvp_val16_unal(&tp, BSSGP_IE_BVCI));
if (bvci == 0) {
diff --git a/src/gbproxy/gb_proxy_peer.c b/src/gbproxy/gb_proxy_peer.c
index ea5fe1ee..79ea8e33 100644
--- a/src/gbproxy/gb_proxy_peer.c
+++ b/src/gbproxy/gb_proxy_peer.c
@@ -165,7 +165,7 @@ struct gbproxy_peer *gbproxy_peer_by_lac(struct gbproxy_config *cfg,
struct gbproxy_peer *gbproxy_peer_by_bssgp_tlv(struct gbproxy_config *cfg,
struct tlv_parsed *tp)
{
- if (TLVP_PRESENT(tp, BSSGP_IE_BVCI)) {
+ if (TLVP_PRES_LEN(tp, BSSGP_IE_BVCI, 2)) {
uint16_t bvci;
bvci = ntohs(tlvp_val16_unal(tp, BSSGP_IE_BVCI));
@@ -174,7 +174,7 @@ struct gbproxy_peer *gbproxy_peer_by_bssgp_tlv(struct gbproxy_config *cfg,
}
/* FIXME: this doesn't make sense, as RA can span multiple peers! */
- if (TLVP_PRESENT(tp, BSSGP_IE_ROUTEING_AREA)) {
+ if (TLVP_PRES_LEN(tp, BSSGP_IE_ROUTEING_AREA, 6)) {
uint8_t *rai = (uint8_t *)TLVP_VAL(tp, BSSGP_IE_ROUTEING_AREA);
/* Only compare LAC part, since MCC/MNC are possibly patched.
* Since the LAC of different BSS must be different when
@@ -183,7 +183,7 @@ struct gbproxy_peer *gbproxy_peer_by_bssgp_tlv(struct gbproxy_config *cfg,
}
/* FIXME: this doesn't make sense, as LA can span multiple peers! */
- if (TLVP_PRESENT(tp, BSSGP_IE_LOCATION_AREA)) {
+ if (TLVP_PRES_LEN(tp, BSSGP_IE_LOCATION_AREA, 5)) {
uint8_t *lai = (uint8_t *)TLVP_VAL(tp, BSSGP_IE_LOCATION_AREA);
return gbproxy_peer_by_lac(cfg, lai);
}