[Bug]: Remotely reachable stack buffer overflow due to the use of unishox2_decompress_simple

[xnyhps]

Category

Other

Hardware

Not Applicable

Firmware Version

2.3.7.30fbcab

Description

There is a stack buffer overflow when decompressing ATAK messages here:

https://github.com/meshtastic/firmware/blob/75dc8cccecd52da78f1d69eeb4eb017fbc68f26c/src/modules/AtakPluginModule.cpp#L114 https://github.com/meshtastic/firmware/blob/75dc8cccecd52da78f1d69eeb4eb017fbc68f26c/src/modules/AtakPluginModule.cpp#L119 https://github.com/meshtastic/firmware/blob/75dc8cccecd52da78f1d69eeb4eb017fbc68f26c/src/modules/AtakPluginModule.cpp#L126 https://github.com/meshtastic/firmware/blob/75dc8cccecd52da78f1d69eeb4eb017fbc68f26c/src/modules/AtakPluginModule.cpp#L133

The function unishox2_decompress_simple does not take the size of the output buffer into account, always assuming it has enough space for the result. Therefore, if an ATAK message contains a compressed field that decompresses into more bytes than is reserved in a meshtastic_TAKPacket, then this will write out of the bounds of that uncompressed field, overwriting other values on the stack (potentially even a pushed return address).

This is actually the same issue as what triggered #3573, except there it was compression that triggered the overflow. The user who reported it sent a base64-encoded message near the maximum size. Due to the high entropy of the message the unishox2 compression actually increased its length from 220 to 245 bytes, also causing an out of bounds write on the stack. Before #3606 was merged it looks like decompression there was affected too with received messages.

The correct function to call would be unishox2_decompress/unishox2_compress and making sure that UNISHOX_API_WITH_OUTPUT_LEN is defined as 1. (#3606 disabled the use of unishox2 for "app" messages, but I'm not sure if not doing the same in the ATAK plugin was an oversight.)

This is a vulnerability that could be exploited by sending a malicious message to a Meshtastic node (the ATAK plugin does not need to actively used just sending a message of this type on a channel the node is on is enough). In the firmware I looked at (the LilyGo T3-S3), this would not be directly exploitable to gain RCE due to the use of stack cookies, but I don't know if that's the case for all supported hardware. Even without knowing the stack cookie value, this can be used to repeatedly crash a node.

Relevant log output

No response

[xnyhps]

I just had a closer look to determine if the firmware for all targets has stack canaries enabled. I found that for 2.3.10.d19607b Beta all ARM based boards do not have stack canaries:

$ grep -L stack_chk_fail *.elf
firmware-canaryone-2.3.10.d19607b.elf
firmware-feather_diy-2.3.10.d19607b.elf
firmware-monteops_hw1-2.3.10.d19607b.elf
firmware-nano-g2-ultra-2.3.10.d19607b.elf
firmware-pca10059_diy_eink-2.3.10.d19607b.elf
firmware-pico-2.3.10.d19607b.elf
firmware-picow-2.3.10.d19607b.elf
firmware-rak11310-2.3.10.d19607b.elf
firmware-rak4631-2.3.10.d19607b.elf
firmware-rak4631_eink-2.3.10.d19607b.elf
firmware-rp2040-lora-2.3.10.d19607b.elf
firmware-senselora_rp2040-2.3.10.d19607b.elf
firmware-t-echo-2.3.10.d19607b.elf

This includes some pretty popular devices: RAK4631 and T-Echo. The xtensa (ESP32) targets all had stack canaries enabled.

If you want to reproduce this, these are the steps I followed to prepare the device sending out a malicious message:

• Disable the ATAK plugin (otherwise these messages get double compressed).
• Check out the Python meshtastic bindings.
• Modify the atak.proto file to change the to field of a GeoChat message to bytes, instead of string (so it doesn't need to be valid UTF8).
• Regenerated the protobufs.
• Ran the following script:

import meshtastic
import meshtastic.serial_interface
import unishox2

iface = meshtastic.serial_interface.SerialInterface(devPath="/dev/cu.usbmodem(...)")

compressed_data, original_size = unishox2.compress(" " * 119 + "\x00" + "AAA" + "C")

geochat = meshtastic.atak_pb2.GeoChat(message="hi", to=compressed_data)
pckt = meshtastic.atak_pb2.TAKPacket(chat=geochat)
pckt.is_compressed = True

print(pckt)
print(pckt.SerializeToString())

iface.sendData(pckt, channelIndex=0, portNum=72)

I found that unishox2 compresses a sequence a lot of spaces to 4 bytes, regardless of how many there are. This also means that it's impossible to guess based on the length of a message how long the decompressed message will be.

This issue is in fact exploitable on a number of devices to gain full control over that device simply by sending a message, so I recommend treating this as a vulnerability.

[thebentern]

@xnyhps I have a PR to resolve this. If you feel like testing your script against it, I would appreciate it. https://github.com/meshtastic/firmware/pull/4317

[xnyhps]

I've looked over the changes and I think this correctly fixes the vulnerability. Thank you for addressing it!

The only thing that looks a bit odd to me is the various checks for the return value to be less than 0:

https://github.com/meshtastic/firmware/blob/33eb073535c2bece2353b9fd76d0dda5d120ac05/src/modules/AtakPluginModule.cpp#L71-L74

As far as I can tell, the unishox2_compress_lines/unishox2_decompress_lines do not return a negative length if the string would overflow. I don't see it documented and looking over the code it looks like instead they return the output length + 1:

https://github.com/meshtastic/firmware/blob/33eb073535c2bece2353b9fd76d0dda5d120ac05/src/mesh/compression/unishox2.cpp#L1393

This isn't a vulnerability, it just means that the output may be truncated instead of bailing out.