From efdd641c295189d99f16c6c37a207fc54027f30f Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Tue, 12 Jan 2021 18:07:18 +0100 Subject: 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 --- tests/tlv/tlv_test.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) (limited to 'tests/tlv/tlv_test.c') diff --git a/tests/tlv/tlv_test.c b/tests/tlv/tlv_test.c index 925d7628..3b4f441c 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + 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) == OSMO_TLVP_ERR_OFS_LEN_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 2, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + OSMO_ASSERT(TLVP_VAL(&dec, 0x23) == NULL); + OSMO_ASSERT(tlv_parse(&dec, &tdef, buf, 1, 0, 0) == OSMO_TLVP_ERR_OFS_BEYOND_BUFFER); + 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; -- cgit v1.2.3