CVE-2020-12887: Integer Overflow or Wraparound (CWE-190)
value: Memory leaks were discovered in the CoAP library in Arm Mbed OS 5.15.3 when using the Arm mbed-coap library 5.1.5. The CoAP parser is responsible for parsing received CoAP packets. The function sn_coap_parser_options_parse() parses the CoAP option number field of all options present in the input packet. Each option number is calculated as a sum of the previous option number and a delta of the current option. The delta and the previous option number are expressed as unsigned 16-bit integers. Due to lack of overflow detection, it is possible to craft a packet that wraps the option number around and results in the same option number being processed again in a single packet. Certain options allocate memory by calling a memory allocation function. In the cases of COAP_OPTION_URI_QUERY, COAP_OPTION_URI_PATH, COAP_OPTION_LOCATION_QUERY, and COAP_OPTION_ETAG, there is no check on whether memory has already been allocated, which in conjunction with the option number integer overflow may lead to multiple assignments of allocated memory to a single pointer. This has been demonstrated to lead to memory leak by buffer orphaning. As a result, the memory is never freed.
CWE description: The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
CVSS v3 base score: 7.5
Published: 2020-06-18T19:15Z
Repository: https://github.com/mjurczak/mbed-coap
Language: C
Files changed: source/sn_coap_parser.c
Commit date: 2020-05-11 22:46:09 +0200
Commit message: Implemented measures to prevent memory leaks in sn_coap_parser_options_parse().

Added a helper uint16_t addition function with overflow detection. The function is used when calculating the extended length and option delta. The overlow detection is needed to avoid wrap-around of option number or length.
Additional checks in options using sn_coap_parser_options_parse_multiple_options() have been implemented to avoid overwriting of pointers pointing to previously allocated memory.

Diff: diff --git a/source/sn_coap_parser.c b/source/sn_coap_parser.c
index d222de4..a2577cd 100644
--- a/source/sn_coap_parser.c
+++ b/source/sn_coap_parser.c
@@ -258,10 +258,33 @@ static uint32_t sn_coap_parser_options_parse_uint(uint8_t **packet_data_pptr, ui
         value |= *(*packet_data_pptr)++;
     }
     return value;
 }
 
+/**
+ * \brief Add u16 integers with overflow detection
+ *
+ * \param a            first term of addition
+ * \param b            second term of addion
+ * \param result       pointer to the result variable
+ *
+ * \return Return 0 if there was no overflow, -1 otherwise
+ */
+static int8_t sn_coap_parser_add_u16_limit(uint16_t a, uint16_t b, uint16_t *result)
+{
+    uint16_t c;
+
+    c = a + b;
+    if (c < a || c < b)
+    {
+        return -1;
+    }
+
+    *result = c;
+
+    return 0;
+}
 
 /**
  * \brief Performs data packet pointer boundary check
  *
  * \param packet_data_ptr           current data packet read pointer
@@ -395,15 +418,19 @@ static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_
             /* packet_data_pptr would overflow! */
             tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow !");
             return -1;
         }
         else {
-                option_number += option_ext;
-                *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
-                                                               packet_data_start_ptr,
-                                                               packet_len,
-                                                               1);
+            if(sn_coap_parser_add_u16_limit(option_number, option_ext, &option_number) != 0)
+            {
+                return -1;
+            }
+
+            *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
+                                                            packet_data_start_ptr,
+                                                            packet_len,
+                                                            1);
         }
     } else if (option_number == 14) {
             int8_t read_result = sn_coap_parser_read_packet_u16(&option_number,
                                                                 *packet_data_pptr,
                                                                 packet_data_start_ptr,
@@ -412,15 +439,19 @@ static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_
                 /* packet_data_pptr would overflow! */
                 tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow !");
                 return -1;
             }
             else {
-            option_number += 269;
-            *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
-                                                           packet_data_start_ptr,
-                                                           packet_len,
-                                                           2);
+                if(sn_coap_parser_add_u16_limit(option_number, 269, &option_number) != 0)
+                {
+                    return -1;
+                }
+
+                *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
+                                                            packet_data_start_ptr,
+                                                            packet_len,
+                                                            2);
             }
     }
     /* Option number 15 reserved for payload marker. This is handled as a error! */
     else if (option_number == 15) {
         tr_error("sn_coap_parser_options_parse - invalid option number(15)!");
@@ -497,11 +528,14 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                                                 &message_left);
         if (option_parse_result != 0) {
             return -1;
         }
         /* Add previous option to option delta and get option number */
-        option_number += previous_option_number;
+        if(sn_coap_parser_add_u16_limit(option_number, previous_option_number, &option_number) != 0)
+        {
+            return -1;
+        }
 
         /* Add possible option length extension to resolve full length of the option */
         option_parse_result = parse_ext_option(&option_len,
                                                 packet_data_pptr,
                                                 packet_data_start_ptr,
@@ -575,10 +609,15 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                 }
                 message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr, packet_data_start_ptr, packet_len, option_len);
                 break;
 
             case COAP_OPTION_ETAG:
+                if (dst_coap_msg_ptr->options_list_ptr->etag_ptr)
+                {
+                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_ETAG exists!");
+                    return -1;
+                }
                 /* This is managed independently because User gives this option in one character table */
                 ret_status = sn_coap_parser_options_parse_multiple_options(handle, packet_data_pptr,
                              message_left,
                              &dst_coap_msg_ptr->options_list_ptr->etag_ptr,
                              (uint16_t *)&dst_coap_msg_ptr->options_list_ptr->etag_len,
@@ -626,10 +665,15 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                 }
                 dst_coap_msg_ptr->options_list_ptr->uri_port = sn_coap_parser_options_parse_uint(packet_data_pptr, option_len);
                 break;
 
             case COAP_OPTION_LOCATION_QUERY:
+                if (dst_coap_msg_ptr->options_list_ptr->location_query_ptr)
+                {
+                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_LOCATION_QUERY exists!");
+                    return -1;
+                }
                 ret_status = sn_coap_parser_options_parse_multiple_options(handle, packet_data_pptr, message_left,
                              &dst_coap_msg_ptr->options_list_ptr->location_query_ptr, &dst_coap_msg_ptr->options_list_ptr->location_query_len,
                              COAP_OPTION_LOCATION_QUERY, option_len);
                 if (ret_status < 0) {
                     tr_error("sn_coap_parser_options_parse - COAP_OPTION_LOCATION_QUERY not valid!");
@@ -637,10 +681,15 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                 }
 
                 break;
 
             case COAP_OPTION_URI_PATH:
+                if (dst_coap_msg_ptr->uri_path_ptr)
+                {
+                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_URI_PATH exists!");
+                    return -1;
+                }
                 ret_status = sn_coap_parser_options_parse_multiple_options(handle, packet_data_pptr, message_left,
                              &dst_coap_msg_ptr->uri_path_ptr, &dst_coap_msg_ptr->uri_path_len,
                              COAP_OPTION_URI_PATH, option_len);
                 if (ret_status < 0) {
                     tr_error("sn_coap_parser_options_parse - COAP_OPTION_URI_PATH not valid!");

Vulnerable code:                 option_number += option_ext;
                *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
                                                               packet_data_start_ptr,
                                                               packet_len,
                                                               1);
            option_number += 269;
            *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
                                                           packet_data_start_ptr,
                                                           packet_len,
                                                           2);
        option_number += previous_option_number;
Fixed code: /**
 * \brief Add u16 integers with overflow detection
 *
 * \param a            first term of addition
 * \param b            second term of addion
 * \param result       pointer to the result variable
 *
 * \return Return 0 if there was no overflow, -1 otherwise
 */
static int8_t sn_coap_parser_add_u16_limit(uint16_t a, uint16_t b, uint16_t *result)
{
    uint16_t c;

    c = a + b;
    if (c < a || c < b)
    {
        return -1;
    }

    *result = c;

    return 0;
}
            if(sn_coap_parser_add_u16_limit(option_number, option_ext, &option_number) != 0)
            {
                return -1;
            }

            *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
                                                            packet_data_start_ptr,
                                                            packet_len,
                                                            1);
                if(sn_coap_parser_add_u16_limit(option_number, 269, &option_number) != 0)
                {
                    return -1;
                }

                *message_left = sn_coap_parser_move_packet_ptr(packet_data_pptr,
                                                            packet_data_start_ptr,
                                                            packet_len,
                                                            2);
        if(sn_coap_parser_add_u16_limit(option_number, previous_option_number, &option_number) != 0)
        {
            return -1;
        }
                if (dst_coap_msg_ptr->options_list_ptr->etag_ptr)
                {
                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_ETAG exists!");
                    return -1;
                }
                if (dst_coap_msg_ptr->options_list_ptr->location_query_ptr)
                {
                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_LOCATION_QUERY exists!");
                    return -1;
                }
                if (dst_coap_msg_ptr->uri_path_ptr)
                {
                    tr_error("sn_coap_parser_options_parse - COAP_OPTION_URI_PATH exists!");
                    return -1;
                }