MSPv1-over-telemetry accepts a 2-byte start frame although the parser reads a 3-byte header

[jFriedli]

Current Behavior

The MSP-over-telemetry parser defines separate minimum sizes:

enum {
    MIN_LENGTH_CHUNK = 2,
    MIN_LENGTH_REQUEST_V1 = 3,
    MIN_LENGTH_REQUEST_V2 = 6,
};

handleMspFrame() only applies the generic two-byte minimum before parsing the start frame:

if (payloadLength < MIN_LENGTH_CHUNK) {
    return false;
}

For an MSPv1 start frame, it then immediately reads the MSP command byte at index 2:

mspPayloadSize = frameStart[MSP_INDEX_SIZE_V1];
requestPacket->cmd = frameStart[MSP_INDEX_ID_V1];

and initializes the request frame with a payload start at index 3:

sbufInit(
    &mspPackage.requestFrame,
    frameStart + MSP_INDEX_PAYLOAD_V1,
    frameStart + payloadLength
);

For payloadLength == 2, the parser therefore:

1. accepts the frame because it satisfies MIN_LENGTH_CHUNK,
2. reads frameStart[2] even though the declared frame length is two bytes,
3. constructs an sbuf_t whose start pointer is frameStart + 3 while its end pointer is frameStart + 2.

Later, the result of sbufBytesRemaining() is stored in uint8_t:

const uint8_t payloadIncoming =
    sbufBytesRemaining(&mspPackage.requestFrame);

Affected source in the tested commit:

• https://github.com/iNavFlight/inav/blob/c5c593d71d33c8e284bf9cd34381588fda7a98c8/src/main/telemetry/msp_shared.c#L772-L804
• https://github.com/iNavFlight/inav/blob/c5c593d71d33c8e284bf9cd34381588fda7a98c8/src/main/telemetry/msp_shared.c#L896-L946
• https://github.com/iNavFlight/inav/blob/c5c593d71d33c8e284bf9cd34381588fda7a98c8/src/main/telemetry/msp_shared.c#L1007-L1036

Security impact

This is a memory-safety / parser-boundary issue in MSP-over-telemetry handling.

A malformed MSPv1 start fragment with a declared logical length of two bytes passes the generic minimum-length check even though the parser immediately consumes fields requiring at least three bytes and constructs an sbuf_t whose start pointer can be beyond its end pointer.

Because this parser handles externally supplied telemetry/MSP fragments, malformed protocol input can drive the flight-controller parser into invalid memory/length state.

Potential consequences include:

• out-of-bounds reads,
• invalid length calculations,
• flight-controller crash or reset,
• denial of service,
• and potentially additional memory-safety effects in downstream processing.

I have not demonstrated controlled code execution or a physical FC crash.

Attacker preconditions

The attacker or peer must be able to inject an MSP-over-telemetry frame that INAV accepts from the configured transport. Whether that capability is available to an arbitrary RF attacker depends on the radio/telemetry system and its pairing/authentication properties.

Relation to #11209 / #11210

This is not intended as a duplicate of the previously fixed CRSF outer-frame length bug.

The earlier issue validated the outer CRSF frame length before subtracting the CRSF overhead. This report concerns a later validation layer inside handleMspFrame(): an MSPv1 logical start frame can be only two bytes long even though the MSPv1 parser requires at least three.

What is proven

• The mismatch between MIN_LENGTH_CHUNK = 2 and MIN_LENGTH_REQUEST_V1 = 3 is present in the tested source.
• The MSPv1 path reads index 2 without first enforcing MIN_LENGTH_REQUEST_V1.
• The resulting request-frame pointer ordering can become invalid.
• A minimal host-side ASan harness demonstrates the logical OOB access when the logical frame is backed by an exact-size allocation.

What is not proven

• That the first OOB logical read necessarily crosses the physical backing allocation on every telemetry transport.
• A physical flight-controller crash.
• Reliable code execution.

The security concern is therefore the malformed external input reaching invalid parser state, not a claim that every two-byte frame immediately produces the exact ASan crash shown by the host harness.

Steps to Reproduce

I reproduced the missing MSPv1 minimum-length check in a host-side AddressSanitizer harness.

Important limitation: this harness allocates exactly two bytes, so ASan makes the index-2 read immediately visible as an allocation-boundary violation.

On a real telemetry transport, the two-byte logical frame may reside inside a larger receive buffer. In that case the immediate CPU memory access may still land inside the backing allocation while being outside the declared frame. The invalid sbuf_t bounds and subsequent signed-to-uint8_t length conversion remain relevant and should be validated in the real transport path.

This has not been reproduced on a physical flight controller.

Save as poc_inav_mspv1_short_start.c:

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

enum {
    TELEMETRY_MSP_VER_MASK = 0x60,
    TELEMETRY_MSP_START_MASK = 0x10,
    TELEMETRY_MSP_VER_SHIFT = 5,
    MIN_LENGTH_CHUNK = 2,
    MSP_INDEX_STATUS = 0,
    MSP_INDEX_SIZE_V1 = 1,
    MSP_INDEX_ID_V1 = 2,
    MSP_INDEX_PAYLOAD_V1 = 3,
};

static void vulnerable_handle(uint8_t *frameStart, int payloadLength)
{
    if (payloadLength < MIN_LENGTH_CHUNK) {
        return;
    }

    const uint8_t status = frameStart[MSP_INDEX_STATUS];
    const uint8_t version =
        (status & TELEMETRY_MSP_VER_MASK) >> TELEMETRY_MSP_VER_SHIFT;

    if ((status & TELEMETRY_MSP_START_MASK) && version == 1) {
        uint16_t mspPayloadSize = frameStart[MSP_INDEX_SIZE_V1];
        uint16_t cmd = frameStart[MSP_INDEX_ID_V1];

        printf("size=%u cmd=%u payload starts at +%d\n",
               mspPayloadSize, cmd, MSP_INDEX_PAYLOAD_V1);
    }
}

int main(void)
{
    uint8_t *frame = malloc(2);
    if (!frame) {
        return 2;
    }

    frame[0] =
        TELEMETRY_MSP_START_MASK |
        (1u << TELEMETRY_MSP_VER_SHIFT);
    frame[1] = 0;

    vulnerable_handle(frame, 2);

    free(frame);
    return 0;
}

Compile and run:

gcc -O0 -g -fsanitize=address,undefined     poc_inav_mspv1_short_start.c     -o poc_inav_mspv1_short_start

ASAN_OPTIONS=abort_on_error=1 ./poc_inav_mspv1_short_start

Observed locally:

ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1
0 bytes after 2-byte region

The tested INAV tree was:

commit: c5c593d71d33c8e284bf9cd34381588fda7a98c8
date:   2026-07-18 22:18:56 -0500
subject: Merge pull request #11728 from iNavFlight/release/9.1

Expected behavior

An MSPv1 start frame should be rejected unless it contains at least the complete MSPv1 telemetry header before any of those fields are accessed.

requestFrame.ptr should never be initialized beyond requestFrame.end.

Suggested solution(s)

Apply the already-defined MSPv1 minimum before parsing the MSPv1 fields:

if (lastRequestVersion == 1) {
    if (payloadLength < MIN_LENGTH_REQUEST_V1) {
        return false;
    }

    ...
}

It would also be useful to avoid narrowing sbufBytesRemaining() to uint8_t until the value is known to be non-negative and within range.

Additional context