Skip to content

[Bug Report] Vulnerable Bug Found in CrsfParser_TryParseCrsfPacket which can trigger Global Buffer Overflow

Low
mrpollo published GHSA-qpw7-65ww-wj82 Nov 13, 2023

Package

No package listed

Affected versions

1.14.0-rc2

Patched versions

1.14.0

Description

Summary

We identified a global buffer overflow vulnerability in the CrsfParser_TryParseCrsfPacket function in /src/drivers/rc/crsf_rc/CrsfParser.cpp:298 due to the invalid size check.

We investigated by the following process

  1. Enable the crsf_rc driver
  2. Write the data (i.e., packet) to _rcs_buf used in the crsf_rc driver on a device
  3. Global buffer overflow vulnerability in the CrsfParser_TryParseCrsfPacket function will be triggered when the CrsfParser_TryParseCrsfPacket function parses this data.

The following packet can trigger the vulnerability as mentioned above.

packet[0] = 0xc8; // MAGIC NUMBER which informs rc packet start
packet[1] = 62; // size of the packet
packet[2] = 100; // packet type ( random, not the 0x16, 0x14 )

Detailed Root Cause

We first assumed that we can write the arbitrary data into _rcs_buf using the following code.

  • /src/drivers/rc/crsf_rc/CrsfRc.cpp #190 line
int new_bytes = ::read(_rc_fd, &_rcs_buf[0], RC_MAX_BUFFER_SIZE);

After that, CrsfParser_TryParseCrsfPacket will be called and parse the information of the rc packet in _rcs_buf.

  • The CrsfParser_TryParseCrsfPacket function checks the following conditions
  1. The first byte of the packet is same with CRSF_HEADER (0xc8). (CrsfParser.cpp:256)
case PARSER_STATE_HEADER:
			if (QueueBuffer_Get(&rx_queue, &working_byte)) {
				if (working_byte == CRSF_HEADER) {
	//...omitted...
  1. Whether the type of the packet is known or not.
    • If the packet’s type is not 0x16 or 0x14, then the working_segment_size sets packet_size(our input) + PACKET_SIZE_TYPE(2);
  2. The working_segment_size should be less or equal than CRSF_MAX_PACKET_LEN which is 64. (if condition in CrsfParser.cpp:298)
    • In that case, if we send the packet which sets size as 62, then working_segment_size will be 64, and this corresponds to the if condition (working_segment_size <= CRSF_MAX_PACKET_LEN). Because the previous condition added 2 with the size of the packet.
    • It makes to pass this if condition, and working_index will set as 2 consequently.
case PARSER_STATE_SIZE_TYPE:
			//PX4_INFO("PARSER_STATE_SIZE_TYPE\n");
			QueueBuffer_Peek(&rx_queue, working_index++, &packet_size); //working_index = 1
			QueueBuffer_Peek(&rx_queue, working_index++, &packet_type); //working_index = 2

			working_descriptor = FindCrsfDescriptor((enum CRSF_PACKET_TYPE)packet_type);

			// If we know what this packet is...
			if (working_descriptor != NULL) {
				//.. the packet's type was 0x16 or 0x14
				//..omitted
				}
			} else {
				// We don't know what this packet is, so we'll let the parser continue
				// just so that we can dequeue it in one shot
				working_segment_size = packet_size + PACKET_SIZE_TYPE_SIZE; //PACKET_SIZE_TYPE_SIZE : 2

				if (working_segment_size > CRSF_MAX_PACKET_LEN) {
					parser_statistics->invalid_unknown_packet_sizes++;
					parser_state = PARSER_STATE_HEADER;
					working_segment_size = HEADER_SIZE;
					working_index = 0;
					buffer_count = QueueBuffer_Count(&rx_queue);
					continue;
				}
			}

			parser_state = PARSER_STATE_PAYLOAD;
			break;
  • Next, in the PARSER_STATE_HEADER part, working_index += working_segment_size is conducted. For our case, working_index is 2 because of the previous parsing part, working_idex will be 2 + 64 = 66.
case PARSER_STATE_PAYLOAD:
			working_index += working_segment_size;
			working_segment_size = CRC_SIZE;
			parser_state = PARSER_STATE_CRC;
			break;
  • The QueueBuffer_PeekBuffer function will be called with the 4th parameter represents size as 67 (because CRC_SIZE is 1)
case PARSER_STATE_CRC:
			// Fetch the suspected packet as a contingous block of memory
			QueueBuffer_PeekBuffer(&rx_queue, 0, process_buffer, working_index + CRC_SIZE);
bool QueueBuffer_PeekBuffer(const QueueBuffer_t *q, const uint32_t index, uint8_t *buffer, const uint32_t size)
{
	uint32_t copy_start;

	//...omitted

	// Check if we can do this in a single shot
	if (copy_start + size <= q->buffer_size) {
		memcpy((void *)buffer, (void *)(q->buffer + copy_start), size); //this will be called

	}
  • Eventually, the data is copied to process_buffer like this (Our case for QueueBuffer.cpp:153)
memcpy(process_buffer, data, 67);

  • But the initialized size of process_buffer is 64

    #define CRSF_MAX_PACKET_LEN 64
static uint8_t process_buffer[CRSF_MAX_PACKET_LEN];

Therefore, the Global buffer overflow will be triggered.

PoC ( Reproduce Method )

We want to reproduce this crash using the actual rc packet, but we just triggered this crash in sitl because we don’t have the actual device yet. So, we’ve tested under the situation we can write the arbitrary data in the device using the crsf_rc driver to receive packets.

So, we suggest the following process to reproduce this packet.

  1. We set the csrf_rc driver in sitl by modifying the configure of default.px4board.

    CONFIG_DRIVERS_RC_CRSF_RC=y

  2. We set the _rcs_buf buffer (in drivers/crsf_rc/CrsfRc.cpp line 190) as below

    • Assuming that the buffer gets the data.
    int new_bytes = ::read(_rc_fd, &_rcs_buf[0], RC_MAX_BUFFER_SIZE);
_rcs_buf[0] = 0xc8; // MAGIC NUMBER which informs rc packet start
_rcs_buf[1] = 62; // size of the packet ( which is vulnerable )
_rcs_buf[2] = 100; // packet type ( random, not the 0x16, 0x14 )
new_bytes = 60 // informs the packet is received

  3. We run the crsf_rc driver like this

    pxh> crsf_rc start -d <device>
    • For example,
    pxh> crsf_rc start -d /dev/ttyS3

Address Sanitizer Log

Untitled

=================================================================
==5090==ERROR: AddressSanitizer: global-buffer-overflow on address 0x563750d9c500 at pc 0x7f15a823a2c3 bp 0x7f15a4606220 sp 0x7f15a46059c8
WRITE of size 67 at 0x563750d9c500 thread T140
ERROR [simulator_mavlink] poll timeout 0, 22
    #0 0x7f15a823a2c2 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x56375022476a in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29
    #2 0x56375022476a in QueueBuffer_PeekBuffer(QueueBuffer_t const*, unsigned int, unsigned char*, unsigned int) /home/ubuntu/drone/PX4-Autopilot/src/drivers/rc/crsf_rc/QueueBuffer.cpp:156
    #3 0x563750223952 in CrsfParser_TryParseCrsfPacket(CrsfPacket_t*, CrsfParserStatistics_t*) /home/ubuntu/drone/PX4-Autopilot/src/drivers/rc/crsf_rc/CrsfParser.cpp:330
    #4 0x56375022126d in CrsfRc::Run() /home/ubuntu/drone/PX4-Autopilot/src/drivers/rc/crsf_rc/CrsfRc.cpp:211
    #5 0x563750a44b00 in px4::WorkQueue::Run() /home/ubuntu/drone/PX4-Autopilot/platforms/common/px4_work_queue/WorkQueue.cpp:188
    #6 0x563750a459bc in WorkQueueRunner /home/ubuntu/drone/PX4-Autopilot/platforms/common/px4_work_queue/WorkQueueManager.cpp:238
    #7 0x7f15a7294b42 in start_thread nptl/pthread_create.c:442
    #8 0x7f15a73269ff  (/lib/x86_64-linux-gnu/libc.so.6+0x1269ff)

0x563750d9c500 is located 32 bytes to the left of global variable 'rx_queue_buffer' defined in '/home/ubuntu/drone/PX4-Autopilot/src/drivers/rc/crsf_rc/CrsfParser.cpp:138:16' (0x563750d9c520) of size 200
0x563750d9c500 is located 0 bytes to the right of global variable 'process_buffer' defined in '/home/ubuntu/drone/PX4-Autopilot/src/drivers/rc/crsf_rc/CrsfParser.cpp:139:16' (0x563750d9c4c0) of size 64
SUMMARY: AddressSanitizer: global-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0ac76a1ab850: f9 f9 f9 f9 00 00 00 00 02 f9 f9 f9 f9 f9 f9 f9
  0x0ac76a1ab860: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0ac76a1ab870: 00 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9
  0x0ac76a1ab880: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0ac76a1ab890: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
=>0x0ac76a1ab8a0:[f9]f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac76a1ab8b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9
  0x0ac76a1ab8c0: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0ac76a1ab8d0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0ac76a1ab8e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac76a1ab8f0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
Thread T140 created by T5 here:
    #0 0x7f15a8258685 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
    #1 0x563750a4608a in WorkQueueManagerRun /home/ubuntu/drone/PX4-Autopilot/platforms/common/px4_work_queue/WorkQueueManager.cpp:324

Thread T5 created by T0 here:
    #0 0x7f15a8258685 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
    #1 0x563750a40aab in px4_task_spawn_cmd /home/ubuntu/drone/PX4-Autopilot/platforms/posix/src/px4/common/tasks.cpp:252

==5090==ABORTING

Recommend Patch

After the line of CrsfParser.cpp:319, the additional limitations of the size will prevent the bug as mentioned before.

case PARSER_STATE_CRC:
			// Fetch the suspected packet as a contingous block of memory
			if(working_index+CRC_SIZE > CRSF_MAX_PACKET_LEN){
				  parser_statistics->invalid_unknown_packet_sizes++;
					parser_state = PARSER_STATE_HEADER;
					working_segment_size = HEADER_SIZE;
					working_index=0;
					buffer_count = QueueBuffer_Count(&rx_queue);
					continue;
			}
			QueueBuffer_PeekBuffer(&rx_queue, 0, process_buffer, working_index + CRC_SIZE);

Impact

If we can create the RC packet remotely and that packet goes into the device where the _rcs_buf reads, the global buffer overflow vulnerability will be triggered so that the drone can behave unexpectedly.

Severity

Low

CVSS overall score

This score calculates overall vulnerability severity from 0 to 10 and is based on the Common Vulnerability Scoring System (CVSS).
/ 10

CVSS v3 base metrics

Attack vector
Local
Attack complexity
High
Privileges required
None
User interaction
None
Scope
Unchanged
Confidentiality
None
Integrity
None
Availability
Low

CVSS v3 base metrics

Attack vector: More severe the more the remote (logically and physically) an attacker can be in order to exploit the vulnerability.
Attack complexity: More severe for the least complex attacks.
Privileges required: More severe if no privileges are required.
User interaction: More severe when no user interaction is required.
Scope: More severe when a scope change occurs, e.g. one vulnerable component impacts resources in components beyond its security scope.
Confidentiality: More severe when loss of data confidentiality is highest, measuring the level of data access available to an unauthorized user.
Integrity: More severe when loss of data integrity is the highest, measuring the consequence of data modification possible by an unauthorized user.
Availability: More severe when the loss of impacted component availability is highest.
CVSS:3.1/AV:L/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:L

CVE ID

CVE-2023-47625

Weaknesses

Credits