fix(core/wire): a truncated trigger datagram stops cancelling the effect it should carry

Three wire and ABI faults.

An out-of-range pad index reached one rumble consumer and not the other. It
skipped the reorder gate — the per-pad seq cursor has no slot for it — and was
handed to the legacy queue, while the policy engine discarded it on its own
bounds check, so the comment promising both consumers are fed was false for
exactly these. An embedder draining the queue could be handed an index it would
use to subscript its own per-pad array. The host never emits one, so it is
malformed or hostile either way; both consumers now agree by dropping it before
either sees it.

The adaptive-trigger effect was the only variable-length wire field bounded on
neither side. Encode appended whatever it was handed and decode took the whole
tail, while its sibling raw-report field had been bounded both ways all along;
there is now one constant both sides clamp to. Worse than the missing bound was
the empty case: a body with no effect bytes decoded as an EMPTY effect, and
downstream an empty block is written as an all-zero trigger report, which is
mode 0x00 — release. A truncated datagram could therefore silently cancel the
trigger effect a game was holding. That shape is now rejected outright; a
genuine release is a full-length zero block and still decodes.

The C ABI history had a hole and a symbol nobody versioned. v11 shipped without
its line, and the rumble policy engine's C surface was added while the version
constant still read 7, with no bump at all — so every core since has exported
those symbols while advertising a number that never promised them. A shipped
binary says what it says, so that cannot be corrected backwards; v15 instead
establishes the floor that guarantees the surface, and the v11 line is written
down. No code changed for the bump and nothing moved on the wire.
This commit is contained in:
2026-08-04 20:52:44 +02:00
parent 454fa2e0cb
commit 77ddd05b13
3 changed files with 127 additions and 16 deletions
@@ -60,22 +60,28 @@ pub(super) async fn run(
}
Some(&crate::quic::RUMBLE_MAGIC) => {
if let Some(u) = crate::quic::decode_rumble_envelope(&d) {
// A pad index the client cannot represent is dropped outright, before either
// consumer sees it. It used to be waved through: the seq gate was skipped (its
// per-pad cursor has no slot for it) and it was handed to the legacy queue,
// while the policy engine silently discarded it on its own bounds check — so
// "both consumers are fed" below was false for exactly these, and an embedder
// draining the queue could be handed an index it would use to subscript its
// own per-pad array. The host never emits one; this is malformed or hostile.
let idx = u.pad as usize;
if idx >= crate::input::MAX_PADS {
continue;
}
// Gate v2 envelopes on their per-pad seq; forward v1 (envelope: None) as-is.
let fresh = match u.envelope {
Some(env) => {
let idx = u.pad as usize;
if idx < crate::input::MAX_PADS {
if crate::input::GamepadSnapshot::seq_newer(
env.seq,
rumble_last_seq[idx],
) {
rumble_last_seq[idx] = Some(env.seq);
true
} else {
false // reordered/duplicate — drop, keep the newer state
}
if crate::input::GamepadSnapshot::seq_newer(
env.seq,
rumble_last_seq[idx],
) {
rumble_last_seq[idx] = Some(env.seq);
true
} else {
true // out-of-range pad (host never sends these): no gate
false // reordered/duplicate — drop, keep the newer state
}
}
None => true,
+13 -1
View File
@@ -107,6 +107,10 @@ pub use stats::Stats;
/// v10: added `punktfunk_connection_clock_offset_now_ns` — the LIVE (mid-stream re-synced)
/// clock offset ongoing latency math must use; the connect-time getter stays frozen by
/// contract. Additive, client-local — no wire change, so [`WIRE_VERSION`] is unchanged.
/// v11: added `punktfunk_connect_ex9` — `connect_ex8` plus a `client_caps` bitfield
/// (`PUNKTFUNK_CLIENT_CAP_CURSOR`, later `…_PHASE_LOCK`), which is how a client tells the host it
/// renders the pointer itself. Additive; the caps ride the existing Hello, so [`WIRE_VERSION`] is
/// unchanged. (Documented late — the bump shipped without its line here.)
/// v12: added `punktfunk_connection_set_cursor_render` — the mid-stream cursor-render flip
/// (design/remote-desktop-sweep.md §8): the client's mouse-model chord tells the host who
/// renders the pointer. Additive; rides the existing control stream (a new message TYPE, which
@@ -120,7 +124,15 @@ pub use stats::Stats;
/// uncertainty and the circular arrival-lead statistic the host's controller steers on. Additive;
/// the wire grows only a new control message (`PhaseReport`, 0x32) an old host never reads and a
/// strict-prefix append on the 0xCF host-timing tail, so [`WIRE_VERSION`] is unchanged.
pub const ABI_VERSION: u32 = 14;
/// v15: versions the shared rumble policy engine's C surface —
/// `punktfunk_connection_next_rumble_cmd`, `punktfunk_connection_set_rumble_quirks` and the
/// `PUNKTFUNK_RUMBLE_QUIRK_*` bits. These symbols are NOT new: they landed while this constant
/// still read 7 and no bump was made, so every core since has exported them while advertising a
/// version that never promised them. That cannot be corrected retroactively — a shipped binary
/// says what it says — so v15 is the floor that *guarantees* them: at or above it the surface is
/// present, below it an embedder must probe for the symbol. Purely a version statement; no code
/// changed with this bump, and no wire change, so [`WIRE_VERSION`] is unchanged.
pub const ABI_VERSION: u32 = 15;
/// The punktfunk/1 **wire** version — what `Hello`/`Welcome` carry and hosts equality-check.
/// Deliberately its own constant: [`ABI_VERSION`] tracks the embeddable **C surface**
+96 -3
View File
@@ -401,6 +401,16 @@ impl RichInput {
}
}
/// Longest [`HidOutput::Trigger`] `effect` the wire carries: the DualSense adaptive-trigger
/// parameter block is a mode byte plus ten parameters, and every consumer copies at most this many
/// into its report.
///
/// The single source for the clamp on BOTH sides. `Trigger` was the only variable-length variant
/// bounded on neither: encode appended whatever it was handed and decode took the entire tail, so
/// an attacker-sized datagram was reproduced verbatim into a `Vec` while its sibling `HidRaw` had
/// been bounded on both ends all along.
pub const TRIGGER_EFFECT_MAX: usize = 11;
const HIDOUT_LED: u8 = 0x01;
const HIDOUT_PLAYER_LEDS: u8 = 0x02;
const HIDOUT_TRIGGER: u8 = 0x03;
@@ -460,7 +470,7 @@ impl HidOutput {
}
HidOutput::Trigger { pad, which, effect } => {
out.extend_from_slice(&[HIDOUT_TRIGGER, *pad, *which]);
out.extend_from_slice(effect);
out.extend_from_slice(&effect[..effect.len().min(TRIGGER_EFFECT_MAX)]);
}
HidOutput::TrackpadHaptic {
pad,
@@ -497,10 +507,17 @@ impl HidOutput {
pad: b[2],
bits: b[3],
}),
HIDOUT_TRIGGER if b.len() >= 4 => Some(HidOutput::Trigger {
// `> 4`, not `>= 4`: a body with no effect bytes at all is malformed, and decoding it
// as an EMPTY effect was actively harmful — downstream an empty block is written as an
// all-zero trigger report, which is mode 0x00, which RELEASES a held effect. A
// truncated datagram could therefore silently cancel the trigger a game was holding.
// A genuine "no effect" is a full-length zero block and still decodes fine.
HIDOUT_TRIGGER if b.len() > 4 => Some(HidOutput::Trigger {
pad: b[2],
which: b[3],
effect: b[4..].to_vec(),
// Bounded like `HidRaw` below: at most the parameter block is kept from the
// (attacker-sized) tail.
effect: b[4..b.len().min(4 + TRIGGER_EFFECT_MAX)].to_vec(),
}),
HIDOUT_TRACKPAD_HAPTIC if b.len() >= 10 => Some(HidOutput::TrackpadHaptic {
pad: b[2],
@@ -981,6 +998,82 @@ mod tests {
assert!(decode_rumble_datagram(&d[..6]).is_none());
}
/// `Trigger` is the only variable-length variant that used to be bounded on NEITHER side.
/// Pinned here because both halves matter: an over-long effect must be clamped on the way out
/// AND on the way in, and a body with no effect bytes must not decode at all.
#[test]
fn trigger_effect_is_clamped_on_both_encode_and_decode() {
// Encode clamps: a caller handing over an over-long block cannot put it on the wire.
let long = HidOutput::Trigger {
pad: 1,
which: 0,
effect: vec![0xAB; 200],
};
let d = long.encode();
assert_eq!(
d.len(),
4 + TRIGGER_EFFECT_MAX,
"magic + kind + pad + which + at most the parameter block"
);
// Decode clamps independently of encode — a hostile peer does not use our encoder.
let mut hostile = vec![HIDOUT_MAGIC, super::HIDOUT_TRIGGER, 1, 0];
hostile.extend_from_slice(&[0xCD; 500]);
match HidOutput::decode(&hostile) {
Some(HidOutput::Trigger { effect, .. }) => {
assert_eq!(effect.len(), TRIGGER_EFFECT_MAX, "tail is bounded");
}
other => panic!("expected a clamped Trigger, got {other:?}"),
}
// An exact-length effect survives untouched, and round-trips.
let ok = HidOutput::Trigger {
pad: 2,
which: 1,
effect: vec![0x02, 0x90, 0xA0, 0xFF, 0, 0, 0, 0, 0, 0, 0],
};
assert_eq!(HidOutput::decode(&ok.encode()), Some(ok));
}
/// A body with no effect bytes is malformed and must be REJECTED, not read as an empty effect:
/// downstream an empty block becomes an all-zero trigger report, which is mode 0x00 — it
/// releases whatever effect the game was holding. A truncated datagram must not do that.
#[test]
fn a_trigger_with_no_effect_bytes_is_rejected_not_read_as_cancel() {
let empty = [HIDOUT_MAGIC, super::HIDOUT_TRIGGER, 0, 0];
assert_eq!(HidOutput::decode(&empty), None);
// One byte of effect is a legitimate short block (consumers zero-pad it) and still decodes.
let one = [HIDOUT_MAGIC, super::HIDOUT_TRIGGER, 0, 0, 0x02];
assert_eq!(
HidOutput::decode(&one),
Some(HidOutput::Trigger {
pad: 0,
which: 0,
effect: vec![0x02]
})
);
}
/// `HidRaw`'s bound was already correct on both sides — pinned alongside `Trigger` so the pair
/// cannot drift apart again.
#[test]
fn hid_raw_stays_bounded_on_both_sides() {
let long = HidOutput::HidRaw {
pad: 0,
kind: HID_RAW_OUTPUT,
data: vec![0x11; 500],
};
assert_eq!(long.encode().len(), 4 + HID_REPORT_MAX);
let mut hostile = vec![HIDOUT_MAGIC, super::HIDOUT_HID_RAW, 0, HID_RAW_FEATURE];
hostile.extend_from_slice(&[0x22; 900]);
match HidOutput::decode(&hostile) {
Some(HidOutput::HidRaw { data, .. }) => assert_eq!(data.len(), HID_REPORT_MAX),
other => panic!("expected a clamped HidRaw, got {other:?}"),
}
}
#[test]
fn rumble_envelope_roundtrip_and_legacy_tolerance() {
// v2 envelope round-trips seq + ttl.