diff options
| author | Neels Hofmeyr <neels@hofmeyr.de> | 2018-03-13 01:22:01 +0100 |
|---|---|---|
| committer | Neels Hofmeyr <neels@hofmeyr.de> | 2018-03-13 01:29:04 +0100 |
| commit | fa10eda6d70a4ecb37b6dfe629e665a16f8d3f59 (patch) | |
| tree | 8bc2188d25bf2093f2b158e15b264c39bb3cdc3b | |
| parent | 620ba9369d63c25774975ea0a903b78929e6420c (diff) | |
vlr_ciph_result: fix use after free of imeisv
Define the struct vlr_ciph_result member .imeisv not as a char* but a char[] of
appropriate length, to avoid the need to point to external memory.
Thus fix a use-after-free in msc_cipher_mode_compl(), which defined the
imeisv[] buffer in a sub-scope within that function, so that the .imeisv
pointer was already invalid when fed to vlr_subscr_rx_ciph_res().
Did you notice that the commit summary rhymes?
Closes: OS#3053
Change-Id: I90cfb952a7dec6d104200872164ebadb25d0260d
| -rw-r--r-- | include/osmocom/msc/vlr.h | 2 | ||||
| -rw-r--r-- | src/libmsc/osmo_msc.c | 4 | ||||
| -rw-r--r-- | src/libvlr/vlr_access_req_fsm.c | 2 | ||||
| -rw-r--r-- | src/libvlr/vlr_lu_fsm.c | 2 |
4 files changed, 4 insertions, 6 deletions
diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h index c4b8cf60c..37702a9ee 100644 --- a/include/osmocom/msc/vlr.h +++ b/include/osmocom/msc/vlr.h @@ -74,7 +74,7 @@ enum vlr_ciph_result_cause { struct vlr_ciph_result { enum vlr_ciph_result_cause cause; - const char *imeisv; + char imeisv[GSM48_MI_SIZE]; }; enum vlr_subscr_security_context { diff --git a/src/libmsc/osmo_msc.c b/src/libmsc/osmo_msc.c index f6df0d201..323baf9aa 100644 --- a/src/libmsc/osmo_msc.c +++ b/src/libmsc/osmo_msc.c @@ -173,7 +173,6 @@ void msc_cipher_mode_compl(struct gsm_subscriber_connection *conn, unsigned int payload_len = msgb_l3len(msg) - sizeof(*gh); struct tlv_parsed tp; uint8_t mi_type; - char imeisv[GSM48_MI_SIZE] = ""; if (!gh) { LOGP(DRR, LOGL_ERROR, "invalid: msgb without l3 header\n"); @@ -187,10 +186,9 @@ void msc_cipher_mode_compl(struct gsm_subscriber_connection *conn, mi_type = TLVP_VAL(&tp, GSM48_IE_MOBILE_ID)[0] & GSM_MI_TYPE_MASK; if (mi_type == GSM_MI_TYPE_IMEISV && TLVP_LEN(&tp, GSM48_IE_MOBILE_ID) > 0) { - gsm48_mi_to_string(imeisv, sizeof(imeisv), + gsm48_mi_to_string(ciph_res.imeisv, sizeof(ciph_res.imeisv), TLVP_VAL(&tp, GSM48_IE_MOBILE_ID), TLVP_LEN(&tp, GSM48_IE_MOBILE_ID)); - ciph_res.imeisv = imeisv; } } } diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c index 95a618da5..3845f2638 100644 --- a/src/libvlr/vlr_access_req_fsm.c +++ b/src/libvlr/vlr_access_req_fsm.c @@ -500,7 +500,7 @@ static void proc_arq_vlr_fn_w_ciph(struct osmo_fsm_inst *fi, } - if (res.imeisv) { + if (*res.imeisv) { LOGPFSM(fi, "got IMEISV: %s\n", res.imeisv); vlr_subscr_set_imeisv(vsub, res.imeisv); } diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c index c6fd080f8..9a4a23912 100644 --- a/src/libvlr/vlr_lu_fsm.c +++ b/src/libvlr/vlr_lu_fsm.c @@ -1165,7 +1165,7 @@ static void lu_fsm_wait_ciph(struct osmo_fsm_inst *fi, uint32_t event, return; } - if (res.imeisv) { + if (*res.imeisv) { LOGPFSM(fi, "got IMEISV: %s\n", res.imeisv); vlr_subscr_set_imeisv(vsub, res.imeisv); } |