Merge pull request #10823 from kb2ma/nanocoap/verify_before_write_option

net/nanocoap: validate option length

sys/net/application_layer/nanocoap/nanocoap.c

@@ -499,7 +499,35 @@ static size_t _encode_uint(uint32_t *val)
     return size;
 }
 
-static unsigned _put_delta_optlen(uint8_t *buf, unsigned offset, unsigned shift, unsigned val)
+/*
+ * Writes CoAP Option header. Expected to be called twice to write an option:
+ *
+ * 1. write delta, using offset 1, shift 4
+ * 2. write length, using offset n, shift 0, where n is the return value from
+ *    the first invocation of this function
+ *
+ *     0   1   2   3   4   5   6   7
+ *   +---------------+---------------+
+ *   |  Option Delta | Option Length |   1 byte
+ *   +---------------+---------------+
+ *   /         Option Delta          /   0-2 bytes
+ *   \          (extended)           \
+ *   +-------------------------------+
+ *   /         Option Length         /   0-2 bytes
+ *   \          (extended)           \
+ *   +-------------------------------+
+ *
+ *   From RFC 7252, Figure 8
+ *
+ * param[out] buf       addr of byte 0 of header
+ * param[in]  offset    offset from buf to write any extended header
+ * param[in]  shift     bit shift for byte 0 value
+ * param[in]  value     delta/length value to write to header
+ *
+ * return     offset from byte 0 of next byte to write
+ */
+static unsigned _put_delta_optlen(uint8_t *buf, unsigned offset, unsigned shift,
+                                  unsigned val)
 {
     if (val < 13) {
         *buf |= (val << shift);
@@ -689,8 +717,14 @@ static ssize_t _add_opt_pkt(coap_pkt_t *pkt, uint16_t optnum, uint8_t *val,
             ? pkt->options[pkt->options_len - 1].opt_num : 0;
     assert(optnum >= lastonum);
 
-    size_t optlen = coap_put_option(pkt->payload, lastonum, optnum, val, val_len);
-    assert(pkt->payload_len > optlen);
+    /* calculate option length */
+    uint8_t dummy[3];
+    size_t optlen = _put_delta_optlen(dummy, 1, 4, optnum - lastonum);
+    optlen += _put_delta_optlen(dummy, 0, 0, val_len);
+    optlen += val_len;
+    assert(pkt->payload_len >= optlen);
+
+    coap_put_option(pkt->payload, lastonum, optnum, val, val_len);
 
     pkt->options[pkt->options_len].opt_num = optnum;
     pkt->options[pkt->options_len].offset = pkt->payload - (uint8_t *)pkt->hdr;

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

@@ -330,6 +330,29 @@ static void test_nanocoap__get_multi_query(void)
     TEST_ASSERT_EQUAL_STRING((char *)qs, &query[1]);
 }
 
+/*
+ * Builds on get_req test, to test building a PDU that completely fills the
+ * buffer.
+ */
+static void test_nanocoap__option_add_buffer_max(void)
+{
+    uint8_t buf[70];    /* header 4, token 2, path 64 */
+    coap_pkt_t pkt;
+    uint16_t msgid = 0xABCD;
+    uint8_t token[2] = {0xDA, 0xEC};
+    char path[] = "/23456789012345678901234567890123456789012345678901234567890123";
+
+    size_t uri_opt_len = 64;    /* option hdr 2, option value 62 */
+
+    size_t len = coap_build_hdr((coap_hdr_t *)&buf[0], COAP_TYPE_NON,
+                                &token[0], 2, COAP_METHOD_GET, msgid);
+
+    coap_pkt_init(&pkt, &buf[0], sizeof(buf), len);
+
+    len = coap_opt_add_string(&pkt, COAP_OPT_URI_PATH, &path[0], '/');
+    TEST_ASSERT_EQUAL_INT(uri_opt_len, len);
+}
+
 /*
  * Helper for server_get tests below.
  * GET Request for nanocoap server example /riot/value resource.
@@ -531,6 +554,7 @@ Test *tests_nanocoap_tests(void)
         new_TestFixture(test_nanocoap__get_path_too_long),
         new_TestFixture(test_nanocoap__get_query),
         new_TestFixture(test_nanocoap__get_multi_query),
+        new_TestFixture(test_nanocoap__option_add_buffer_max),
         new_TestFixture(test_nanocoap__server_get_req),
         new_TestFixture(test_nanocoap__server_reply_simple),
         new_TestFixture(test_nanocoap__server_get_req_con),