src: codec: Add MAVLink2 signing, resync policy, and exploit tests - #11
Draft
joaoantoniocardoso wants to merge 6 commits into
Draft
src: codec: Add MAVLink2 signing, resync policy, and exploit tests#11joaoantoniocardoso wants to merge 6 commits into
joaoantoniocardoso wants to merge 6 commits into
Conversation
Add an exploits test binary organized as one module folder, with each of the six exploit groups a module and each scenario its own file: packet_in_packet, forgery_drops, desync_liveness, robustness, parser_dos, and trust_model, plus a shared common harness. Covers packet-in-packet injection (incl. the len=1 bypass), forgery via sysid/compid/incompat/unknown-msgid drops, version downgrade (CVE-2020-10283), desync/liveness, panic-safety, parser forward-progress (ArduPilot #32120), the skip-CRC trust boundary (CVE-2020-10282), and a signed-frame CRC-offset finding. Security-spec tests are expected to fail until the packet-in-packet resync is fixed (cf. rust-mavlink PR #508).
Add a 7th const-generic VERIFY_SIGNATURE to MavlinkCodec plus an Option<SigningData> field and a with_signing() constructor. When enabled, v2 frames are HMAC-verified after the CRC check and rejected with the new DecoderError::InvalidSignature; v1 frames are rejected outright since they cannot be signed. Signature verification is the only mechanism that actually closes the packet-in-packet vulnerability: a planted inner frame cannot carry a valid MAC, so it is rejected rather than decoded. Unsigned v2 and all v1 frames remain inherently vulnerable (residual risk documented in the exploit suite). Also fix v2::checksum() to locate the CRC from the payload length instead of the full packet size, which was wrong for signed frames (it pointed into the signature block). Enable the mavlink "signing" feature, extend the test type aliases with the new parameter, convert the v2 packet-in-packet exploits to signed codecs (now mitigated), and mark the v1 cases ignored as residual risk.
MAVLink frames are length-delimited, so on rejection the parser now skips
the full declared frame via a new CodecState::Discarding { remaining } arm
(chunk-safe across reads) instead of advancing a single byte and rescanning
for the next STX.
This closes packet-in-packet for every rejection path where the inner frame
sits within the outer's declared length (oversized, sysid/compid policy
drop, unknown msgid, incompat flags, cross-version nesting): the inner is
discarded together with the rejected outer rather than resynced onto. It
aligns the codec with the rest of the MAVLink ecosystem (c_library_v2,
MAVSDK, pymavlink, libmav, gomavlib, mavlink-router), matching the direction
of rust-mavlink PR #508.
The deliberate trade-off is over-skip: a frame that lies about its length
(e.g. len=1) is still trusted, so the len=1 packet-in-packet remains residual
and is only closed by signature verification. Leading byte-by-byte STX
resync is kept for the unsynced state to preserve forward progress.
Update the exploit suite: scenarios where the inner is inside the declared
length now assert the inner is not decoded (renamed to
inner_discarded_with_outer); len=1 stays as the residual demonstration.
Build the rust-mavlink raw message straight from the frame slice via a new raw_v2_from_slice helper, dropping the intermediate Bytes allocation. Full zero-copy is blocked upstream (private secret key and a MAVLinkV2MessageRaw-only verify API).
Record the deliberate security default on CodecState::Discarding and in a new SECURITY.md: discarding the whole declared frame closes packet-in-packet injection, at a liveness cost. A forward-rescan toggle was considered and rejected as it reopens the injection vector.
This was referenced Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add protocol-exploitation tests, MAVLink2 signature verification, whole-frame discard resync, and SECURITY.md.
Made with Cursor