osmith has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31262 )
Change subject: gsm0808_dec_channel_type: add missing len check ......................................................................
gsm0808_dec_channel_type: add missing len check
Stop iterating if the extension bit (0x80) is set but elem is too short to read another byte.
Related: OS#4393 Change-Id: Id37109dba0f5d40f4b83f0cef9b1dbd9d6bb2c68 --- M src/gsm/gsm0808_utils.c M tests/gsm0808/gsm0808_test.c 2 files changed, 15 insertions(+), 0 deletions(-)
Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved
diff --git a/src/gsm/gsm0808_utils.c b/src/gsm/gsm0808_utils.c index 9d787f2..83d2ce7 100644 --- a/src/gsm/gsm0808_utils.c +++ b/src/gsm/gsm0808_utils.c @@ -550,6 +550,9 @@ elem++;
for (i = 0; i < ARRAY_SIZE(ct->perm_spch); i++) { + if (elem - old_elem >= len) + return -EOVERFLOW; + byte = *elem; elem++; ct->perm_spch[i] = byte & 0x7f; diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c index ab9f29b..f68d560 100644 --- a/tests/gsm0808/gsm0808_test.c +++ b/tests/gsm0808/gsm0808_test.c @@ -1117,6 +1117,17 @@ msgb_free(msg); }
+static void test_gsm0808_dec_channel_type_err(void) +{ + struct gsm0808_channel_type ct; + int rc; + + /* Speech: extension bit is set in last byte */ + const uint8_t hex[] = { 0x01, 0x0b, 0xa1, 0xa5 }; + rc = gsm0808_dec_channel_type(&ct, hex, sizeof(hex)); + OSMO_ASSERT(rc == -EOVERFLOW); +} + static void test_gsm0808_enc_dec_encrypt_info(void) { struct gsm0808_encrypt_info enc_ei = { @@ -2569,6 +2580,7 @@ test_gsm0808_enc_dec_speech_codec_list(); test_gsm0808_enc_dec_empty_speech_codec_list(); test_gsm0808_enc_dec_channel_type(); + test_gsm0808_dec_channel_type_err(); test_gsm0808_enc_dec_encrypt_info();
test_gsm0808_enc_dec_cell_id_list_lac();