fixeria has uploaded this change for review.

View Change

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)

To view, visit change 43244. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: newchange
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I3140319c6ab5dabb2e356f5c59f6cb05542554cd
Gerrit-Change-Number: 43244
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>