osmocom
/
libosmocore
mirror of https://gerrit.osmocom.org/libosmocore
11
0
Fork 0
Code Issues Releases Wiki Activity

tlv_parser: Fix various out-of-bounds accesses 

The libosmocore TLV parser had a number of insufficient bounds checks
leading to reads beyond the end of the respective input buffer.

This patch
* adds proper out-of-bounds checks to all TLV types
* simplifies some of the existing checks
* introduces test cases to test all the corner cases
  where either TAG, or length, or value are not fully contained
  in the input buffer.

Thanks to Ilja Van Sprundel for reporting these problems.

Change-Id: I98b02c914c9e3ecf56050af846292aa6979d7508
This commit is contained in:
Harald Welte 2021-01-12 18:07:18 +01:00
parent 5681a3254c
commit 96c61074f5
3 changed files with 115 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/gsm/tlv_parser.c
View File

@ -231,7 +231,10 @@ int tlv_parse_one(uint8_t *o_tag, uint16_t *o_len, const uint8_t **o_val,
const uint8_t *buf, int buf_len)
{
uint8_t tag;
int len;
int len; /* number of bytes consumed by TLV entry */
if (buf_len < 1)
return -1;
tag = *buf;
*o_tag = tag;
@ -264,56 +267,54 @@ int tlv_parse_one(uint8_t *o_tag, uint16_t *o_len, const uint8_t **o_val,
break;
case TLV_TYPE_TLV:
tlv: /* GSM TS 04.07 11.2.4: Type 4 TLV */
if (buf + 1 > buf + buf_len)
if (buf_len < 2)
return -1;
*o_val = buf+2;
*o_len = *(buf+1);
len = *o_len + 2;
if (len > buf_len)
return -2;
break;
case TLV_TYPE_vTvLV_GAN: /* 44.318 / 11.1.4 */
/* FIXME: variable-length TAG! */
if (buf_len < 2)
return -1;
if (*(buf+1) & 0x80) {
/* like TL16Vbut without highest bit of len */
if (2 > buf_len)
if (buf_len < 3)
return -1;
/* like TL16Vbut without highest bit of len */
*o_val = buf+3;
*o_len = (*(buf+1) & 0x7F) << 8 | *(buf+2);
len = *o_len + 3;
if (len > buf_len)
return -2;
} else {
/* like TLV */
goto tlv;
}
break;
case TLV_TYPE_TvLV:
if (buf_len < 2)
return -1;
if (*(buf+1) & 0x80) {
/* like TLV, but without highest bit of len */
if (buf + 1 > buf + buf_len)
return -1;
*o_val = buf+2;
*o_len = *(buf+1) & 0x7f;
len = *o_len + 2;
if (len > buf_len)
return -2;
break;
}
/* like TL16V, fallthrough */
case TLV_TYPE_TL16V:
if (2 > buf_len)
if (buf_len < 3)
return -1;
*o_val = buf+3;
*o_len = *(buf+1) << 8 | *(buf+2);
len = *o_len + 3;
if (len > buf_len)
return -2;
break;
default:
return -3;
}
if (buf_len < len) {
*o_val = NULL;
return -2;
}
return len;
}

92
tests/tlv/tlv_test.c
View File

@ -332,6 +332,97 @@ static void test_tlv_encoder()
msgb_free(msg);
}
static void test_tlv_parser_bounds()
{
struct tlv_definition tdef;
struct tlv_parsed dec;
uint8_t buf[32];
memset(&tdef, 0, sizeof(tdef));
printf("Testing TLV_TYPE_T decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_T;
buf[0] = 0x23;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == 1);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 0, 0, 0) == 0);
printf("Testing TLV_TYPE_TV decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_TV;
buf[0] = 0x23;
buf[1] = 0x42;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == 1);
OSMO_ASSERT(*TLVP_VAL(&dec, 0x23) == buf[1]);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
printf("Testing TLV_TYPE_FIXED decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_FIXED;
tdef.def[0x23].fixed_len = 2;
buf[0] = 0x23;
buf[1] = 0x42;
buf[2] = 0x55;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 3, 0, 0) == 1);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
printf("Testing TLV_TYPE_TLV decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_TLV;
buf[0] = 0x23;
buf[1] = 0x02;
buf[2] = 0x55;
buf[3] = 0xAA;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 4, 0, 0) == 1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == &buf[2]);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 3, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
printf("Testing TLV_TYPE_vTvLV_GAN decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_vTvLV_GAN;
buf[0] = 0x23;
buf[1] = 0x80;
buf[2] = 0x01;
buf[3] = 0xAA;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 4, 0, 0) == 1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == &buf[3]);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 3, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
printf("Testing TLV_TYPE_TvLV decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_TvLV;
buf[0] = 0x23;
buf[1] = 0x81;
buf[2] = 0xAA;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 3, 0, 0) == 1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == &buf[2]);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
printf("Testing TLV_TYPE_TL16V decoder for out-of-bounds\n");
tdef.def[0x23].type = TLV_TYPE_TL16V;
buf[0] = 0x23;
buf[1] = 0x00;
buf[2] = 0x01;
buf[3] = 0xAA;
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 4, 0, 0) == 1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == &buf[3]);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 3, 0, 0) == -2);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == -1);
OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL);
}
int main(int argc, char **argv)
{
//osmo_init_logging2(ctx, &info);
@ -339,6 +430,7 @@ int main(int argc, char **argv)
test_tlv_shift_functions();
test_tlv_repeated_ie();
test_tlv_encoder();
test_tlv_parser_bounds();
printf("Done.\n");
return EXIT_SUCCESS;

7
tests/tlv/tlv_test.ok
View File

@ -1,4 +1,11 @@
Test shift functions
Testing TLV encoder by decoding + re-encoding binary
Testing TLV encoder with IE ordering
Testing TLV_TYPE_T decoder for out-of-bounds
Testing TLV_TYPE_TV decoder for out-of-bounds
Testing TLV_TYPE_FIXED decoder for out-of-bounds
Testing TLV_TYPE_TLV decoder for out-of-bounds
Testing TLV_TYPE_vTvLV_GAN decoder for out-of-bounds
Testing TLV_TYPE_TvLV decoder for out-of-bounds
Testing TLV_TYPE_TL16V decoder for out-of-bounds
Done.