Memory leak in MbedOS CoAP library parser - sn_coap_parser_options_parse()

[mjurczak]

Description of defect

References:

https://github.com/ARMmbed/mbed-os/tree/mbed-os-5.15.3/features/frameworks/mbed-coap

https://github.com/ARMmbed/mbed-coap/tree/v5.1.5

File:

sn_coap_parser.c

Example Trace (64 Byte leak):

MALLOC: allocated 44 bytes @ 0xf4c00f50, current depth: 1.
Allocation depth before parsing: 1
MALLOC: allocated 44 bytes @ 0xf4c00f10, current depth: 2.
MALLOC: allocated 64 bytes @ 0xf5a01640, current depth: 3.
MALLOC: allocated 1 bytes @ 0xf48007b0, current depth: 4.
FREE: freed buffer @ 0xf48007b0, current depth: 3.
FREE: freed buffer @ 0xf4c00f10, current depth: 2.
Allocation depth after parsing and releasing: 2

=================================================================
ERROR: detected memory leaks

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0xf7a06c17 in malloc
    #1 0x5659c669 in coap_malloc main.c
    #2 0x565a9945 in sn_coap_parser_options_parse_multiple_options sn_coap_parser.c
    #3 0x565a7d41 in sn_coap_parser_options_parse sn_coap_parser.c
    #4 0x565a3ec4 in sn_coap_parser sn_coap_parser.c
    #5 0x5659ca9a in main main.c
    #6 0xf7726ee4 in __libc_start_main

SUMMARY: 64 byte(s) leaked in 1 allocation(s).

Example Trace (double 8 Byte leak):

MALLOC: allocated 44 bytes @ 0xf4d00f50, current depth: 1.
Allocation depth before parsing: 1
MALLOC: allocated 44 bytes @ 0xf4d00f10, current depth: 2.
MALLOC: allocated 68 bytes @ 0xf4901ad0, current depth: 3.
MALLOC: allocated 8 bytes @ 0xf47007b0, current depth: 4.
MALLOC: allocated 8 bytes @ 0xf4700790, current depth: 5.
MALLOC: allocated 1 bytes @ 0xf4700770, current depth: 6.
FREE: freed buffer @ 0xf4700770, current depth: 5.
FREE: freed buffer @ 0xf4901ad0, current depth: 4.
FREE: freed buffer @ 0xf4d00f10, current depth: 3.
Allocation depth after parsing and releasing: 3

=================================================================
ERROR: detected memory leaks

Direct leak of 16 byte(s) in 2 object(s) allocated from:
    #0 0xf7a42c17 in malloc 
    #1 0x56639669 in coap_malloc main.c
    #2 0x56646945 in sn_coap_parser_options_parse_multiple_options sn_coap_parser.c
    #3 0x56643eb8 in sn_coap_parser_options_parse sn_coap_parser.c
    #4 0x56640ec4 in sn_coap_parser sn_coap_parser.c
    #5 0x56639a9a in main main.c
    #6 0xf7762ee4 in __libc_start_main

SUMMARY: 16 byte(s) leaked in 2 allocation(s).

Analysis:

If a packet with multiple options with the same effective option number, but with non-zero delta is processed it may lead to memory leak.

This issue is related to:
#12930

The parser assumes in sn_coap_parser_options_parse_multiple_options() that options with the same number can occur only adjacent to each other using zero delta after the first option with a given number.
This is not true due to integer overflow in option number addition which makes it possible to craft a packet with multiple options resulting in the same option number in arbitrary order.
In conjunction with lack of verification of pointers to allocated memory before rewriting the pointer with newly allocated space it may lead to memory leak due to memory buffer orphaning.

Integer overflow is described in detail in:
#12930

uint16 variable overflow can happen at:

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines 309 to 322 in b6370b4

	if (option_number == 13) {
	option_number = *(*packet_data_pptr + 1) + 13;
	(*packet_data_pptr)++;
	} else if (option_number == 14) {
	if (message_left >= 2){
	option_number = *(*packet_data_pptr + 2);
	option_number += (*(*packet_data_pptr + 1) << 8) + 269;
	(*packet_data_pptr) += 2;
	} else {
	/* packet_data_pptr would overflow! */
	tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow !");
	return -1;
	}
	}

and

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Line 338 in b6370b4

option_number += previous_option_number;

If more than one occurence of option with the same number is encountered by sn_coap_parser_options_parse(), the sn_coap_parser_options_parse_multiple_options() may allocate new memory buffer and overwrite pointer pointing to previously allocated memory space in:

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines 434 to 441 in b6370b4

	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,
	&dst_coap_msg_ptr->options_list_ptr->etag_ptr,
	(uint16_t *)&dst_coap_msg_ptr->options_list_ptr->etag_len,
	COAP_OPTION_ETAG, option_len);

or

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines 497 to 500 in b6370b4

	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);

or

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines 510 to 513 in b6370b4

	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);

or

mbed-os/features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines 535 to 538 in b6370b4

	case COAP_OPTION_URI_QUERY:
	ret_status = sn_coap_parser_options_parse_multiple_options(handle, packet_data_pptr, message_left,
	&dst_coap_msg_ptr->options_list_ptr->uri_query_ptr, &dst_coap_msg_ptr->options_list_ptr->uri_query_len,
	COAP_OPTION_URI_QUERY, option_len);

As a result the previously allocated buffer is orphaned and never freed.

Patch proposal:
mjurczak/mbed-coap@4647a68

Type:

• Integer Overflow or Wraparound
• Memory leak / heap exhaustion

Result:

• Excessive memory usage
• Possible crash due to lack of resources
• Denial of Service

Target(s) affected by this defect ?

• MbedOS mbed-coap library 5.1.5
• MbedOS 5.15.3

Toolchain(s) (name and version) displaying this defect ?

N/A

What version of Mbed-os are you using (tag or sha) ?

MbedOS 5.15.3

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

N/A

How is this defect reproduced ?

Parsing the provided input example input with sn_coap_parser() function.

#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include "sn_coap_protocol.h"
#include "sn_coap_header.h"

struct coap_s* coapHandle;
coap_version_e coapVersion = COAP_VERSION_1;

static int32_t allocation_counter = 0;

void* coap_malloc(uint16_t size){
    void *ptr;
    ptr = malloc(size);
    allocation_counter++;
    printf("MALLOC: allocated %d bytes @ %p, current depth: %d.\r\n", size, ptr, allocation_counter);
    return ptr;
}

void coap_free(void* addr){
    free(addr);
    if (addr != NULL)
    {
        allocation_counter--;
        printf("FREE: freed buffer @ %p, current depth: %d.\r\n", addr, allocation_counter);
    }
}

uint8_t coap_tx_cb(uint8_t *arg_a, uint16_t arg_b, sn_nsdl_addr_s *arg_c, void *arg_d){
    return 0;
}

int8_t coap_rx_cb(sn_coap_hdr_s *arg_a, sn_nsdl_addr_s *arg_b, void *arg_c){
    return 0;
}

int main(int argc, const char* argv[])
{
    FILE *fp;
    size_t read_bytes;
    size_t input_size;
    uint8_t *message_buffer;
    if (argc != 2)
    {
       return 1;
    }
    
    fp = fopen(argv[1], "r");
    if (fp == NULL)
    {
      return 2;
    }
     
    fseek (fp , 0 , SEEK_END);
    input_size = ftell(fp);
    rewind (fp);
     
    if (input_size > 65527)
    {
      return 3;
    }
     
    message_buffer = malloc(input_size);
    read_bytes = fread(message_buffer, 1, input_size, fp);
    fclose(fp);
    coapHandle = sn_coap_protocol_init(&coap_malloc, &coap_free, &coap_tx_cb, &coap_rx_cb);
    printf("Allocation depth before parsing: %d\r\n", allocation_counter);
    sn_coap_hdr_s* parsed = sn_coap_parser(coapHandle, read_bytes, message_buffer, &coapVersion);
    sn_coap_parser_release_allocated_coap_msg_mem(coapHandle, parsed);
    printf("Allocation depth after parsing and releasing: %d\r\n", allocation_counter);
    free(message_buffer);
    
    return 0;
}

mem_leak_8B_double_option.log
mem_leak_16B_triple_option.log
mem_leak_64B_double_option.log

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2671

[yogpan01]

Hi @mjurczak,
We checked your proposal here and we can definitely take the contribution so if you can please open a Pull Request to http://github.com/ARMmbed/mbed-coap we can officially review and take it in and make it back to mbed-os repo.

Regards,
Yogesh

[mjurczak]

Hi @yogpan01,
A pull-request to mbed-coap repo : PelionIoT/mbed-coap#116