coap: Fix zero-length options

This error prevented me from connecting to leshan.eclipseprojects.io using the DTLS enabled client binary.
eclipse-wakaama · Jan 30, 2022 · 6b72e0f · 6b72e0f
1 parent ca5e599
commit 6b72e0f
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 8 deletions.
diff --git a/coap/er-coap-13/er-coap-13.c b/coap/er-coap-13/er-coap-13.c
@@ -794,39 +794,35 @@ coap_parse_message(void *packet, uint8_t *data, uint16_t data_len)
       goto exit_parse_error;
     }
 
-    if (++current_option == data_end) {
-      goto exit_parse_error;
-    }
-
     /* avoids code duplication without function overhead */
     x = &option_delta;
     do
     {
       if (*x==13)
       {
-        *x += current_option[0];
         if (++current_option == data_end) {
           goto exit_parse_error;
         }
+        *x += current_option[0];
       }
       else if (*x==14)
       {
         *x += 255;
-        *x += current_option[0]<<8;
         if (++current_option == data_end) {
           goto exit_parse_error;
         }
-        *x += current_option[0];
+        *x += current_option[0]<<8;
         if (++current_option == data_end) {
           goto exit_parse_error;
         }
+        *x += current_option[0];
       }
     }
     while (x != &option_length && (x = &option_length));
 
     option_number += option_delta;
 
-    if (current_option + option_length > data_end)
+    if (++current_option + option_length > data_end)
     {
         PRINTF("OPTION %u (delta %u, len %u) has invalid length.\n", option_number, option_delta, option_length);
         goto exit_parse_error;
@@ -841,6 +837,9 @@ coap_parse_message(void *packet, uint8_t *data, uint16_t data_len)
     switch (option_number)
     {
       case COAP_OPTION_CONTENT_TYPE:
+        if(option_length > 2) {
+            goto exit_parse_error;
+        }
         coap_pkt->content_type = (coap_content_type_t) coap_parse_int_option(current_option, option_length);
         PRINTF("Content-Format [%u]\n", coap_pkt->content_type);
         break;

diff --git a/tests/er_coap_coap_parse_message.c b/tests/er_coap_coap_parse_message.c
@@ -245,6 +245,49 @@ static void test_option_format_unsupported_critical(void) {
     CU_ASSERT_EQUAL(coap_parse_message(&coap_pkt, data, sizeof(data)), BAD_OPTION_4_02);
 }
 
+static void test_option_format_content_format_null(void) {
+    uint8_t data[] = {
+        0x60, // version 1, no options, no tokens
+        0x83, // Non-empty message
+        0xC3, // Message ID
+        0xF8, // Message ID
+        0xC0, // Content format "text/plain; charset=utf-8"
+    };
+
+    CU_ASSERT_EQUAL(coap_parse_message(&coap_pkt, data, sizeof(data)), NO_ERROR);
+    CU_ASSERT_PTR_EQUAL(coap_pkt.content_type, 0);
+}
+
+static void test_option_format_content_format_non_null(void) {
+    uint8_t data[] = {
+        0x60, // version 1, no options, no tokens
+        0x83, // Non-empty message
+        0xC3, // Message ID
+        0xF8, // Message ID
+        0xC2, // Content format; option length is 2 byte
+        0xFD, // 1st byte of 0xFDE8 (experimental usage)
+        0xE8, // 2nd byte
+    };
+
+    CU_ASSERT_EQUAL(coap_parse_message(&coap_pkt, data, sizeof(data)), NO_ERROR);
+    CU_ASSERT_PTR_EQUAL(coap_pkt.content_type, 0xFDE8);
+}
+
+static void test_option_format_content_format_invalid(void) {
+    uint8_t data[] = {
+        0x60, // version 1, no options, no tokens
+        0x83, // Non-empty message
+        0xC3, // Message ID
+        0xF8, // Message ID
+        0xC3, // Content format; option length is 3 byte (invalid!)
+        0xFD, // 1st byte of 0x999999 (invalid!)
+        0xE8, // 2nd byte
+        0xE8, // 3nd byte
+    };
+
+    CU_ASSERT_NOT_EQUAL(coap_parse_message(&coap_pkt, data, sizeof(data)), NO_ERROR);
+}
+
 static void test_option_format_unsupported_elective(void) {
     uint8_t data[] = {
         0x40, // version 1, no options, no token
@@ -324,6 +367,9 @@ static struct TestTable table[] = {
     {"Option format: If none match", test_option_format_if_none_match},
     {"Option format: Unsupported, elective", test_option_format_unsupported_elective},
     {"Option format: Unsupported, critical", test_option_format_unsupported_critical},
+    {"Option format: Content format zero", test_option_format_content_format_null},
+    {"Option format: Content format 2 byte size", test_option_format_content_format_non_null},
+    {"Option format: Content format with invalid option length", test_option_format_content_format_invalid},
     {"Payload: Minimal", test_payload_min},
     {"Payload: Maximal", test_payload_max},
     {NULL, NULL},