Skip to content

Instantly share code, notes, and snippets.

@quart27219
Last active July 7, 2026 05:36
Show Gist options
  • Select an option

  • Save quart27219/6bfcc615f89fb493d02aad480704593b to your computer and use it in GitHub Desktop.

Select an option

Save quart27219/6bfcc615f89fb493d02aad480704593b to your computer and use it in GitHub Desktop.
Unchecked MAVLink SERIAL_CONTROL count causes out-of-bounds read in ArduPilot
### Summary
`GCS_MAVLINK::handle_serial_control()` uses the wire-controlled
`SERIAL_CONTROL.count` field as the write length for the fixed-size decoded
`SERIAL_CONTROL.data[70]` buffer without first checking that
`count <= sizeof(data)`.
A decoder-accepted MAVLink `SERIAL_CONTROL` message can therefore cause:
```cpp
stream->write(packet.data, packet.count);
```
to read past the end of the decoded `packet.data` array.
### Root Cause
mavlink_serial_control_t contains a fixed-width data buffer:
`packet.data`
with a maximum serialized data length of 70 bytes.
However, packet.count is a separate field controlled by the received MAVLink
message. After decoding, packet.count can be larger than
sizeof(packet.data).
The handler currently trusts packet.count and passes it directly as the number of bytes to write:
`stream->write(packet.data, packet.count);`
If `packet.count > sizeof(packet.data)`, the stream layer is asked to read
past the end of the decoded packet.data buffer.
### Related Logs
The following Valgrind output is supporting runtime evidence only. It shows
that bytes originating from stack storage in the MAVLink receive path can reach
the SITL UART outbound send path. The root cause is the missing post-decode
bounds check on `packet.count`.
```
==3486== Conditional jump or move depends on uninitialised value(s)
==3486== at 0x3F34C1: mavlink_packetise(ByteBuffer&, unsigned short)
==3486== by 0x35DE51: HALSITL::UARTDriver::_timer_tick()
==3486== Uninitialised value was created by a stack allocation
==3486== at 0x277871: GCS_MAVLINK::raw_packetReceived(
==3486== unsigned char,
==3486== __mavlink_status const&,
==3486== __mavlink_message const&
==3486== )
==3486== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==3486== at 0x4D1CF2A: __libc_send (send.c:28)
==3486== by 0x4D1CF2A: send (send.c:23)
==3486== by 0x35DEC9: HALSITL::UARTDriver::_timer_tick()
==3486== Address 0x1fff0005e6 is on thread 1's stack
==3486== in frame #1, created by HALSITL::UARTDriver::_timer_tick()
==3486== Uninitialised value was created by a stack allocation
==3486== at 0x277871: GCS_MAVLINK::raw_packetReceived(...)
```
### Observed behavior
When the message is handled, handle_serial_control() reaches:
`stream->write(packet.data, packet.count);`
with packet.count == 255 while the decoded `packet.data` field is a fixed 70-byte array.
Runtime memory checking can report that bytes originating from stack storage
are later used in the outbound send path. This is consistent with
`packet.count` causing the stream layer to read beyond the fixed
`packet.data` array.
### Expected behavior
handle_serial_control() should never use packet.count as a buffer length
unless it has first been bounded against sizeof(packet.data).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment