riot-ci/RIOT
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #10754 from kaspar030/fix_nanocoap_optlen

Browse Source 
nanocoap: fix server-side option_count overflow
This commit is contained in:
Kaspar Schleiser 2019-01-14 13:26:37 +01:00 committed by GitHub
commit 9767dd3d49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 92 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
sys/net/application_layer/nanocoap/nanocoap.c
View File

@ -106,6 +106,11 @@ int coap_parse(coap_pkt_t *pkt, uint8_t *buf, size_t len)
DEBUG("option count=%u nr=%u len=%i\n", option_count, option_nr, option_len);
if (option_delta) {
if (option_count >= NANOCOAP_NOPTS_MAX) {
DEBUG("nanocoap: max nr of options exceeded\n");
return -ENOMEM;
}
optpos->opt_num = option_nr;
optpos->offset = (uintptr_t)option_start - (uintptr_t)hdr;
DEBUG("optpos option_nr=%u %u\n", (unsigned)option_nr, (unsigned)optpos->offset);

87
tests/unittests/tests-nanocoap/tests-nanocoap.c
View File

@ -15,6 +15,7 @@
#include <stdint.h>
#include <stdbool.h>
#include <string.h>
#include <stdio.h>
#include "embUnit.h"
@ -433,6 +434,90 @@ static void test_nanocoap__server_reply_simple_con(void)
TEST_ASSERT_EQUAL_INT(COAP_TYPE_ACK, coap_get_type(&pkt));
}
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, 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[] = {
0x40, 0x01, 0x09, 0x26, 0x01, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17, 0x11, 0x17,
0x11, 0x17, 0x11, 0x17 };
/* ensure NANOCOAP_NOPTS_MAX is actually lower than 42 */
TEST_ASSERT(NANOCOAP_NOPTS_MAX < 42);
struct {
coap_pkt_t pkt;
uint8_t guard_data[42 * sizeof(coap_optpos_t)];
} scratch;
memset(&scratch, 0, sizeof(scratch));
int res = coap_parse(&scratch.pkt, pkt_data, sizeof(pkt_data));
/* check if any byte of the guard_data array is non-zero */
int dirty = 0;
volatile uint8_t *pos = scratch.guard_data;
for (size_t i = 0; i < sizeof(scratch.guard_data); i++) {
if (*pos) {
dirty = 1;
break;
}
}
TEST_ASSERT_EQUAL_INT(0, dirty);
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) {
@ -450,6 +535,8 @@ Test *tests_nanocoap_tests(void)
new_TestFixture(test_nanocoap__server_reply_simple),
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);