diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c index 159b42bdd..5640abe86 100644 --- a/src/gsm/tlv_parser.c +++ b/src/gsm/tlv_parser.c @@ -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; } diff --git a/tests/tlv/tlv_test.c b/tests/tlv/tlv_test.c index 925d7628f..148a7c271 100644 --- a/tests/tlv/tlv_test.c +++ b/tests/tlv/tlv_test.c @@ -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; diff --git a/tests/tlv/tlv_test.ok b/tests/tlv/tlv_test.ok index f3f0fd411..e24b889bb 100644 --- a/tests/tlv/tlv_test.ok +++ b/tests/tlv/tlv_test.ok @@ -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.