Skip to content

Commit

Permalink
Implemented measures to prevent memory leaks in sn_coap_parser_option…
Browse files Browse the repository at this point in the history
…s_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.
  • Loading branch information
mjurczak committed May 11, 2020
1 parent 14aff16 commit 4647a68
Showing 1 changed file with 60 additions and 11 deletions.
71 changes: 60 additions & 11 deletions source/sn_coap_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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! */
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 4647a68

Please sign in to comment.