Stack based buffer overflow while parsing MQTT messages (parsing PUBLISH message with variable length header) #600
Description
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 64Overflow: 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)