Vulnerability: 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.
Vulnerability ID: CVE-2020-12887
CWE: CWE-190
Severity score: 2.9
Project: https://github.com/mjurczak/mbed-coap
Published: 2020-06-18
File: source/sn_coap_parser.c
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.
Commit author: Maciej Jurczak

Vulnerable version (before fix):
 static uint32_t sn_coap_parser_options_parse_uint(uint8_t **packet_data_pptr, ui     return value; }   /**  * \brief Performs data packet pointer boundary check static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_             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);         }     } else if (option_number == 14) {             int8_t read_result = sn_coap_parser_read_packet_u16(&option_number, static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_                 return -1;             }             else {             option_number += 269;             *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! */ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack             return -1;         }         /* Add previous option to option delta and get option number */         option_number += previous_option_number;          /* Add possible option length extension to resolve full length of the option */         option_parse_result = parse_ext_option(&option_len, static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack                 break;              case COAP_OPTION_ETAG:                 /* 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, static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack                 break;              case COAP_OPTION_LOCATION_QUERY:                 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); static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack                 break;              case COAP_OPTION_URI_PATH:                 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);

Fixed version (after fix):
 static uint32_t sn_coap_parser_options_parse_uint(uint8_t **packet_data_pptr, ui     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 static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_             return -1;         }         else {             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, static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_                 return -1;             }             else {                 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! */ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack             return -1;         }         /* Add previous option to option delta and get 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, static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack                 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, static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack                 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); 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);

Patch:
patch: @@ -260,6 +260,29 @@ static uint32_t sn_coap_parser_options_parse_uint(uint8_t **packet_data_pptr, ui
     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
@@ -397,11 +420,15 @@ static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_
             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,
@@ -414,11 +441,15 @@ static int8_t parse_ext_option(uint16_t *dst, uint8_t **packet_data_pptr, uint8_
                 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! */
@@ -499,7 +530,10 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
             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,
@@ -577,6 +611,11 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                 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,
@@ -628,6 +667,11 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
                 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);
@@ -639,6 +683,11 @@ 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);
path: source/sn_coap_parser.c