From 1058a8d07e06dc43c857a39c49020dbbc1dcdfcc Mon Sep 17 00:00:00 2001 From: Ken Bannister <kb2ma@runbox.com> Date: Sat, 12 Jan 2019 07:39:39 -0500 Subject: [PATCH] tests/nanocoap: add test for option parse overflow --- .../unittests/tests-nanocoap/tests-nanocoap.c | 50 ++++++++++++++++--- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/tests/unittests/tests-nanocoap/tests-nanocoap.c b/tests/unittests/tests-nanocoap/tests-nanocoap.c index 801561a417..38185b2bbd 100644 --- a/tests/unittests/tests-nanocoap/tests-nanocoap.c +++ b/tests/unittests/tests-nanocoap/tests-nanocoap.c @@ -437,12 +437,13 @@ static void test_nanocoap__server_reply_simple_con(void) static void test_nanocoap__server_option_count_overflow_check(void) { /* this test passes a forged CoAP packet containing 42 options (provided by - * @nmeum in #10753) to coap_parse(). The used coap_pkt_t is part of a - * struct, followed by an array of 42 coap_option_t. The array is cleared - * before the call to coap_parse(). If the overflow protection is working, - * the array must still be clear after parsing the packet, and the proper - * error code (-ENOMEM) is returned. Otherwise, the parsing wrote past - * scratch.pkt, thus the array is not zeroed anymore. + * @nmeum in #10753, 42 is a random number which just needs to be higher + * than NANOCOAP_NOPTS_MAX) to coap_parse(). The used coap_pkt_t is part + * of a struct, followed by an array of 42 coap_option_t. The array is + * cleared before the call to coap_parse(). If the overflow protection is + * working, the array must still be clear after parsing the packet, and the + * proper error code (-ENOMEM) is returned. Otherwise, the parsing wrote + * past scratch.pkt, thus the array is not zeroed anymore. */ static uint8_t pkt_data[] = { @@ -481,6 +482,42 @@ static void test_nanocoap__server_option_count_overflow_check(void) TEST_ASSERT_EQUAL_INT(-ENOMEM, res); } +/* + * Verifies that coap_parse() recognizes inclusion of too many options. + */ +static void test_nanocoap__server_option_count_overflow(void) +{ + /* base pkt is a GET for /riot/value, which results in two options for the + * path, but only 1 entry in the options array. + * Size buf to accept an extra 2-byte option */ + unsigned base_len = 17; + uint8_t buf[17 + (2 * NANOCOAP_NOPTS_MAX)] = { + 0x42, 0x01, 0xbe, 0x16, 0x35, 0x61, 0xb4, 0x72, + 0x69, 0x6f, 0x74, 0x05, 0x76, 0x61, 0x6c, 0x75, + 0x65 + }; + coap_pkt_t pkt; + + /* nonsense filler option that contains a single byte of data */ + uint8_t fill_opt[] = { 0x11, 0x01 }; + + /* fill pkt with maximum options; should succeed */ + int i = 0; + for (; i < (2 * (NANOCOAP_NOPTS_MAX - 1)); i+=2) { + memcpy(&buf[base_len+i], fill_opt, 2); + } + + /* don't read final two bytes, where overflow option will be added later */ + int res = coap_parse(&pkt, buf, sizeof(buf) - 2); + TEST_ASSERT_EQUAL_INT(0, res); + + /* add option to overflow */ + memcpy(&buf[base_len+i], fill_opt, 2); + + res = coap_parse(&pkt, buf, sizeof(buf)); + TEST_ASSERT(res < 0); +} + Test *tests_nanocoap_tests(void) { EMB_UNIT_TESTFIXTURES(fixtures) { @@ -499,6 +536,7 @@ Test *tests_nanocoap_tests(void) new_TestFixture(test_nanocoap__server_get_req_con), new_TestFixture(test_nanocoap__server_reply_simple_con), new_TestFixture(test_nanocoap__server_option_count_overflow_check), + new_TestFixture(test_nanocoap__server_option_count_overflow), }; EMB_UNIT_TESTCALLER(nanocoap_tests, NULL, NULL, fixtures); -- GitLab