Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out of range memory access in MbedOS CoAP library parser - sn_coap_parser_options_parse_multiple_options #12928

Closed
mjurczak opened this issue May 5, 2020 · 2 comments

Comments

@mjurczak
Copy link

mjurczak commented May 5, 2020

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 Traces:
[sn_coap_parser.c:684__read_buffer_overflow.log](https://github.com/ARMmbed/mbed-os/files/4583604/sn_coap_parser.c.684__read_buffer_overflow.log)

    #0  sn_coap_parser_options_parse_multiple_options()
        sn_coap_parser.c:684
    #1  sn_coap_parser_options_parse()
        sn_coap_parser.c:536
    #2  sn_coap_parser()
        sn_coap_parser.c:161       

Analysis:

If a packet with malformed URI-Query option is provided as input, the parser reads out of the provided input packet memory area.

(*packet_data_pptr) += option_number_len;
temp_parsed_uri_query_ptr += option_number_len;
if ((temp_parsed_uri_query_ptr - *dst_pptr) >= uri_query_needed_heap || ((**packet_data_pptr >> COAP_OPTIONS_OPTION_NUMBER_SHIFT) != 0)) {
return returned_option_counter;
}

The packet_data_pptr is accessed after being incremented by option_len without prior out-of-bound memory check.
The temp_parsed_uri_query_ptr is validated for correct range, but the range valid for temp_parsed_uri_query_ptr is derived from the amount of allocated heap memory, not the input size. Therefore the check of temp_parsed_uri_query_ptr may be insufficient for safe access to the area pointed by packet_data_pptr.

int16_t uri_query_needed_heap = sn_coap_parser_options_count_needed_memory_multiple_option(*packet_data_pptr, packet_left_len, option, option_number_len);

Type:

  • Integer Overflow or Wraparound
  • Out-of-bounds Read

Result:

  • Parsing data out of input bounds
  • Possible crash due out-of-bound memory access

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;

void* coap_malloc(uint16_t size){
    return malloc(size);
}

void coap_free(void* addr){
    free(addr);
}

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);
    sn_coap_hdr_s* parsed = sn_coap_parser(coapHandle, read_bytes, message_buffer, &coapVersion);
    sn_coap_parser_release_allocated_coap_msg_mem(coapHandle, parsed);
    free(message_buffer);
    
    return 0;
}

sn_coap_parser.c:684__read_buffer_overflow.log

@ciarmcom
Copy link
Member

ciarmcom commented May 6, 2020

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-2661

@mjurczak
Copy link
Author

mjurczak commented May 8, 2020

Patch proposal:

https://github.com/mjurczak/mbed-coap/tree/bugfix/buffer_read_out_of_bounds

I have also not implemented any more testing measures of correct parser operation than running the unit test suite provided with the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants