fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43244?usp=email )

Change subject: gsm0808: fix buffer overflow in gsm0808_dec_encrypt_info() ......................................................................

gsm0808: fix buffer overflow in gsm0808_dec_encrypt_info()

ei->key_len was set to len - 1 and used as the memcpy() size into ei->key[ENCRY_INFO_KEY_MAXLEN] without checking it against the buffer's actual size. A TLV-parsed Encryption Information IE with len 254 or 255 overflowed key[] by 1-2 bytes into the adjacent key_len field, silently corrupting it and, in turn, the function's own return value.

Change-Id: I3140319c6ab5dabb2e356f5c59f6cb05542554cd Reported-By: Akhil Koul akhil.koul8@gmail.com --- M src/gsm/gsm0808_utils.c M tests/gsm0808/gsm0808_test.c 2 files changed, 30 insertions(+), 0 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/43244/1

diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c index c9f26d3..dd58b64 100644 --- a/src/gsm/gsm0808_utils.c +++ b/src/gsm/gsm0808_utils.c @@ -854,6 +854,8 @@ /* FIXME: 48.008 3.2.2.10 Encryption Information says: * "When present, the key shall be 8 octets long." */ ei->key_len = len - 1; + if (ei->key_len > ENCRY_INFO_KEY_MAXLEN) + return -ENOSPC; memcpy(ei->key, elem, ei->key_len); elem+=ei->key_len;

diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c index ed99245..8bb6817 100644 --- a/tests/gsm0808/gsm0808_test.c +++ b/tests/gsm0808/gsm0808_test.c @@ -1285,6 +1285,34 @@ OSMO_ASSERT(memcmp(&enc_ei, &dec_ei, sizeof(enc_ei)) == 0);

msgb_free(msg); + + /* Maximum permitted key length (perm_algo byte + ENCRY_INFO_KEY_MAXLEN + * key bytes) must still be accepted. */ + { + uint8_t elem[1 + ENCRY_INFO_KEY_MAXLEN]; + elem[0] = 0x03; + memset(elem + 1, 0x42, ENCRY_INFO_KEY_MAXLEN); + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, elem, sizeof(elem)); + OSMO_ASSERT(rc_dec == sizeof(elem)); + OSMO_ASSERT(dec_ei.key_len == ENCRY_INFO_KEY_MAXLEN); + } + + /* A key length exceeding ENCRY_INFO_KEY_MAXLEN by 1 or 2 bytes used to + * overflow dec_ei.key[]; it must now be rejected. */ + { + uint8_t elem[1 + ENCRY_INFO_KEY_MAXLEN + 1]; + elem[0] = 0x03; + memset(elem + 1, 0x42, ENCRY_INFO_KEY_MAXLEN + 1); + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, elem, sizeof(elem)); + OSMO_ASSERT(rc_dec == -ENOSPC); + } + { + uint8_t elem[1 + ENCRY_INFO_KEY_MAXLEN + 2]; + elem[0] = 0x03; + memset(elem + 1, 0x42, ENCRY_INFO_KEY_MAXLEN + 2); + rc_dec = gsm0808_dec_encrypt_info(&dec_ei, elem, sizeof(elem)); + OSMO_ASSERT(rc_dec == -ENOSPC); + } }

static void test_gsm0808_dec_cell_id_list_srvcc(void)