pespin submitted this change.
gsm: Reject BSSMAP Encryption Information IE with key length != 8
From early TS 08.08 to current 3GPP TS 48.008 release 19,
3.2.2.10 Encryption Information IE clearly states that
the key size should be 8 if present.
This also in turn fixes a potential write buffer overflow if an IE with
len>253 was passed to the decoding function.
This patch fixes the problem by rewriting the function, since it
actually was way too convoluted/complex for the kind of checks it had to
do. This way also a call to memcpy is avoided if no key is present.
Related: OS#7054
Reported-By: Adam Bedard <adam.bedard@gmail.com>
Change-Id: I9fd70875f35cebb923278869e1260c5bf3047439
---
M src/gsm/gsm0808_utils.c
M tests/gsm0808/gsm0808_test.c
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c
index 1abfbc8..d7bc275 100644
--- a/src/gsm/gsm0808_utils.c
+++ b/src/gsm/gsm0808_utils.c
@@ -831,7 +831,6 @@
uint8_t perm_algo;
unsigned int i;
unsigned int perm_algo_len = 0;
- const uint8_t *old_elem = elem;
if (!elem)
return -EINVAL;
@@ -854,14 +853,18 @@
}
}
ei->perm_algo_len = perm_algo_len;
-
- /* FIXME: 48.008 3.2.2.10 Encryption Information says:
- * "When present, the key shall be 8 octets long." */
ei->key_len = len - 1;
- memcpy(ei->key, elem, ei->key_len);
- elem+=ei->key_len;
- return (int)(elem - old_elem);
+ /* No key present, done */
+ if (ei->key_len == 0)
+ return len;
+
+ /* "When present, the key shall be 8 octets long." */
+ if (ei->key_len != 8)
+ return -EINVAL;
+ OSMO_ASSERT(sizeof(ei->key) >= ei->key_len);
+ memcpy(ei->key, elem, ei->key_len);
+ return len;
}
/*! Encode TS 48.008 Kc128 IE.
diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 497de6c..8ebbd4b 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -1282,6 +1282,25 @@
rc_dec = gsm0808_dec_encrypt_info(&dec_ei, msg->data + 2, msg->len - 2);
OSMO_ASSERT(rc_dec == 9);
OSMO_ASSERT(memcmp(&enc_ei, &dec_ei, sizeof(enc_ei)) == 0);
+
+
+ /* Test decoding IE with No Encryption and hence with no Key */
+ struct gsm0808_encrypt_info enc_ei_no_encryption = {
+ .perm_algo = { GSM0808_ALG_ID_A5_0 },
+ .perm_algo_len = 1,
+ .key = { 0 },
+ .key_len = 0,
+ };
+ uint8_t ei_enc_no_encryption_expected[] = { GSM0808_IE_ENCRYPTION_INFORMATION, 0x01, 0x01 };
+
+ msg = msgb_alloc(1024, "output buffer");
+ rc_enc = gsm0808_enc_encrypt_info(msg, &enc_ei_no_encryption);
+ OSMO_ASSERT(rc_enc == 3);
+ OSMO_ASSERT(memcmp(ei_enc_no_encryption_expected, msg->data, msg->len) == 0);
+
+ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, msg->data + 2, msg->len - 2);
+ OSMO_ASSERT(rc_dec == 1);
+ OSMO_ASSERT(memcmp(&enc_ei_no_encryption, &dec_ei, sizeof(enc_ei_no_encryption)) == 0);
msgb_free(msg);
/* Test decoding of malformed IE with no algo selected: */
@@ -1289,6 +1308,15 @@
rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_no_algo[2], sizeof(ei_enc_no_algo) - 2);
OSMO_ASSERT(rc_dec == -EINVAL);
+ /* Test decoding of malformed IE with wrong key length: */
+ uint8_t ei_enc_wrong_key_len[256] = { GSM0808_IE_ENCRYPTION_INFORMATION, 0xfd, 0x03 };
+ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_wrong_key_len[2], 0xfd);
+ OSMO_ASSERT(rc_dec == -EINVAL);
+ /* test with another invalid key length: */
+ ei_enc_wrong_key_len[1] = 0x03;
+ rc_dec = gsm0808_dec_encrypt_info(&dec_ei, &ei_enc_wrong_key_len[2], 0x03);
+ OSMO_ASSERT(rc_dec == -EINVAL);
+
}
static void test_gsm0808_dec_cell_id_list_srvcc(void)
To view, visit change 43251. To unsubscribe, or for help writing mail filters, visit settings.