tests/nanocoap: add test for option parse overflow

sys/net/application_layer/nanocoap/nanocoap.c

@@ -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");
+ 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);

tests/unittests/tests-nanocoap/tests-nanocoap.c

@@ -15,6 +15,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <string.h>
+#include <stdio.h>
 
 #include "embUnit.h"
 
@@ -433,6 +434,76 @@ 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)
+{
+ 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 };
+
+ 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));
+
+ 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 +521,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);