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
This commit is contained in:
Neels Hofmeyr 2018-03-13 01:22:01 +01:00
parent 620ba9369d
commit fa10eda6d7
4 changed files with 4 additions and 6 deletions

View File

@ -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 {

View File

@ -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;
}
}
}

View File

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

View File

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