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

Stack based buffer overflow while parsing MQTT messages (parsing PUBLISH message with variable length header) #600

Closed
cve-reporting opened this Issue Jul 11, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@cve-reporting

cve-reporting commented Jul 11, 2018

Function parse_publish_vhdr() that parses MQTT PUBLISH messages with variable length header (see details in: http://www.steves-internet-guide.com/mqtt-protocol-messages-overview/) tries to memcpy input data into fixed size buffer.
Allocated buffer can fit only MQTT_MAX_TOPIC_LENGTH (64) bytes and the length check is missing.

Declaration of buffer:

contiki-ng/os/net/app-layer/mqtt/mqtt.h:226-228
	struct mqtt_message {
	  uint32_t mid;
	  char topic[MQTT_MAX_TOPIC_LENGTH + 1]; /* +1 for string termination */

contiki-ng/os/net/app-layer/mqtt/mqtt.h:111
	#define MQTT_MAX_TOPIC_LENGTH 64

Overflow: contiki-ng/os/net/app-layer/mqtt/mqtt.c:895

    memcpy(&conn->in_publish_msg.topic[conn->in_packet.topic_pos],
           &input_data_ptr[*pos],
           copy_bytes);

This could lead to Remote Code Execution via stack smashing attack (overwriting the function return address).

Proposed CVSS score:

CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H (10.0 - Critical)

Mitigation:
The size of data copied to topic buffer should be limited to MQTT_MAX_TOPIC_LENGTH.

Crash details using Address Sanitizer (line number could not match to original sources):

=================================================================
==19213==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff743a40a0 at pc 0x7fe2a1b68904 bp 0x7fff743a3880 sp 0x7fff743a3028
WRITE of size 63 at 0x7fff743a40a0 thread T0
    #0 0x7fe2a1b68903 in __asan_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x8c903)
    #1 0x402d57 in parse_publish_vhdr contiki-ng/os/net/app-layer/mqtt/mqtt.c:892
    #2 0x402d57 in tcp_input contiki-ng/os/net/app-layer/mqtt/mqtt.c:1002
    #3 0x401367 in main contiki-ng/os/net/app-layer/mqtt/mqtt.:111
    #4 0x7fe2a173282f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #5 0x401528 in _start (
test_mqtt_asan.exe+0x401528)

Address 0x7fff743a40a0 is located in stack of thread T0 at offset 1920 in frame
    #0 0x4011ef in main test_mqtt.c:75

  This frame has 1 object(s):
    [32, 1920) 'conn' <== Memory access at offset 1920 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 __asan_memcpy
Shadow bytes around the buggy address:
  0x10006e86c7c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c7d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c7e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c7f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10006e86c810: 00 00 00 00[f3]f3 f3 f3 f3 f3 f3 f3 00 00 00 00
  0x10006e86c820: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006e86c860: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==19213==ABORTING

Crash details from exploitable gdb plugin:

---CRASH SUMMARY---
Filename: findings_dir_001/crashes/id:000002,sig:11,src:000070,op:havoc,rep:32
SHA1: 00f8ee9a86104bcd9b865420229884922263b318
Classification: EXPLOITABLE
Hash: a1403e50f808c1b633be14d7672c2756.a1403e50f808c1b633be14d7672c2756
Command: ./test_mqtt.exe findings_dir_001/crashes/id:000002,sig:11,src:000070,op:havoc,rep:32
Faulting Frame:
   None @ 0x00007ffff7980000: in ?
Disassembly:
Stack Head (2 entries):
   None                      @ 0x00007ffff7980000: in ?
   None                      @ 0x0000000000000000: in ?
Registers:
rax=0x0000000000000000 rbx=0x8a8d848484848484 rcx=0x0000000000604570 rdx=0x0000000000000000 
rsi=0x0000000000000000 rdi=0x00007ffff7dd1b20 rbp=0x00008484848d8484 rsp=0x00007fffffffd7d0 
 r8=0x2121676e69737261  r9=0x0000000000000001 r10=0x00000000000008b8 r11=0x00007ffff7a914f0 
r12=0x0000000000400cc0 r13=0x00007fffffffd8a0 r14=0xff84000000008084 r15=0x0000000000000000 
rip=0x00007ffff7980000 efl=0x0000000000010202  cs=0x0000000000000033  ss=0x000000000000002b 
 ds=0x0000000000000000  es=0x0000000000000000  fs=0x0000000000000000  gs=0x0000000000000000 
Extra Data:
   Description: Segmentation fault on program counter
   Short description: SegFaultOnPc (3/22)
   Explanation: The target tried to access data at an address that matches the program counter. This is likely due to the execution of a branch instruction (ex: 'call') with a bad argument, but it could also be due to execution continuing past the end of a memory region or another cause. Regardless this likely indicates that the program counter contents are tainted and can be controlled by an attacker.
---END SUMMARY---

[crash_000_parse_publish_vhdr.txt](https://github.com/contiki-ng/contiki-ng/files/2184405/crash_000_parse_publish_vhdr.txt)
[crash_001_parse_publish_vhdr.txt](https://github.com/contiki-ng/contiki-ng/files/2184406/crash_001_parse_publish_vhdr.txt)
[crash_002_parse_publish_vhdr.txt](https://github.com/contiki-ng/contiki-ng/files/2184407/crash_002_parse_publish_vhdr.txt)
@simonduq

This comment has been minimized.

Member

simonduq commented Oct 15, 2018

Thanks a lot for the detailed report. A fix was submitted at #702

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