fix(core): streamed-AU hardening from the security-review pass
apple / swift (push) Successful in 1m19s
docker / deploy-docs (push) Has been cancelled
android / android (push) Has been cancelled
apple / screenshots (push) Has been cancelled
arch / build-publish (push) Has been cancelled
release / apple (push) Successful in 8m41s
ci / rust (push) Has been cancelled
ci / web (push) Has been cancelled
ci / docs-site (push) Has been cancelled
ci / bench (push) Has been cancelled
deb / build-publish (push) Has been cancelled
deb / build-publish-host (push) Has been cancelled
decky / build-publish (push) Has been cancelled
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Has been cancelled
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Has been cancelled
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Has been cancelled
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Has been cancelled
flatpak / build-publish (push) Has been cancelled
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Has been cancelled
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Has been cancelled
windows-host / package (push) Has been cancelled
windows-msix / package (arm64, C:\Users\Public\ffmpeg-arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (push) Has been cancelled
windows-msix / package (x64, C:\Users\Public\ffmpeg, , x86_64-pc-windows-msvc, C:\t) (push) Has been cancelled
windows / build (aarch64-pc-windows-msvc) (push) Has been cancelled
windows / build (x86_64-pc-windows-msvc) (push) Has been cancelled

Review verdict: no exploitable memory-safety / amplification / splice /
resurrection issue; nonce order and two-lane seal byte-identical for the
legacy path. Actionable findings applied:
- explicit per-block data_shards cross-check next to the recovery_shards
  one (defense-in-depth — the geometry invariants make it unreachable
  today, but have_data indexing assumes it)
- partial delivery skips an UNPINNED streamed frame (frame_bytes still 0)
  instead of truncating its max-sized buffer to an empty "partial"
- regression tests for the two untested load-bearing guards: the
  final-first out-of-range-sentinel drop (the exact-sized-buffer guard)
  and second-final-with-different-totals rejection

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-20 23:13:59 +02:00
parent 421e811225
commit 3736bbdc73
2 changed files with 120 additions and 1 deletions
+12 -1
View File
@@ -497,6 +497,14 @@ impl Reassembler {
drop(stats);
return Ok(None);
}
// Defense-in-depth (2026-07 security review): the geometry invariants above guarantee
// every packet of a block agrees on K, so this can't fire today — but `have_data`
// indexing and the recovery-slot math below assume it, and an explicit check keeps a
// future firewall refactor from turning that assumption into an OOB panic.
if block.data_shards != data_shards {
drop(stats);
return Ok(None);
}
if block.done {
// A data shard the parity reconstruct already restored (`!have_data`) was late, not
// lost — net it out of the `fec_recovered_shards` it was counted into (see the
@@ -694,7 +702,10 @@ impl ReassemblyWindow {
// where shards are missing (the codec's block walk skips zero windows).
// Newest-wins if several age out in one prune. Still counted dropped below.
if let Some(sink) = partial_sink.as_deref_mut() {
if f.user_flags & USER_FLAG_CHUNK_ALIGNED != 0 {
// `frame_bytes > 0` also excludes an UNPINNED streamed frame (its total is
// still the 0 sentinel value): truncating its max-sized buffer to 0 would
// deliver an empty "partial" — worse than the plain drop it gets instead.
if f.user_flags & USER_FLAG_CHUNK_ALIGNED != 0 && f.frame_bytes > 0 {
let mut buf = std::mem::take(&mut f.buf);
buf.truncate(f.frame_bytes);
let newer = sink
+108
View File
@@ -931,3 +931,111 @@ fn streamed_open_amplification_is_budget_bounded() {
"the fifth max-sized open must be refused by the in-flight budget"
);
}
/// The final-first order's single buffer-safety guard (2026-07 security review finding 3): a
/// frame opened by its FINAL block allocates an EXACT-sized buffer; a sentinel aimed at (or
/// past) the pinned final slot must be dropped — without the guard its full-K write would land
/// outside that buffer. And the reject must not corrupt the frame: it still completes.
#[test]
fn streamed_out_of_range_sentinel_after_final_first_is_dropped() {
let mut r = Reassembler::new(limits());
let coder = coder_for(FecScheme::Gf8);
let stats = StatsCounters::default();
// Final block opens the frame: block_count = 1, frame_bytes = 32 → K = 2. Send shard 0
// only, so the frame stays in flight with the totals pinned.
let mut fin = base_header();
fin.block_count = 1;
fin.frame_bytes = 32;
fin.data_shards = 2;
fin.recovery_shards = 0;
fin.shard_index = 0;
assert!(r
.push(&packet(fin), coder.as_ref(), &stats)
.unwrap()
.is_none());
// Sentinels at the final slot (0) and past it (1): both non-final-impossible under the
// pinned block_count = 1 → dropped, never written into the 32-byte buffer.
for bi in 0..2u16 {
let mut h = base_header();
h.block_count = 0;
h.frame_bytes = 0;
h.data_shards = 8;
h.recovery_shards = 0;
h.block_index = bi;
assert!(r
.push(&packet(h), coder.as_ref(), &stats)
.unwrap()
.is_none());
}
assert_eq!(stats.snapshot().packets_dropped, 2);
// The frame is unharmed: its real second shard completes it at the exact pinned length.
let mut fin2 = fin;
fin2.shard_index = 1;
let got = r
.push(&packet(fin2), coder.as_ref(), &stats)
.unwrap()
.expect("frame must still complete after the rejected sentinels");
assert_eq!(got.data.len(), 32);
assert!(got.complete);
}
/// A second "final" header with DIFFERENT totals must be rejected once a streamed frame is
/// pinned (re-pinning would re-interpret already-landed shards), and the frame must still
/// complete under the first totals.
#[test]
fn streamed_second_final_with_different_totals_is_rejected() {
let mut r = Reassembler::new(limits());
let coder = coder_for(FecScheme::Gf8);
let stats = StatsCounters::default();
let sentinel_shard = |shard_index: u16| {
let mut h = base_header();
h.block_count = 0;
h.frame_bytes = 0;
h.data_shards = 8;
h.recovery_shards = 0;
h.block_index = 0;
h.shard_index = shard_index;
h
};
let final_shard = |frame_bytes: u32, data_shards: u16, shard_index: u16| {
let mut h = base_header();
h.block_count = 2;
h.frame_bytes = frame_bytes;
h.data_shards = data_shards;
h.recovery_shards = 0;
h.block_index = 1;
h.shard_index = shard_index;
h
};
// Sentinel opens block 0, then the real final pins totals: 10 shards = 160 bytes.
assert!(r
.push(&packet(sentinel_shard(0)), coder.as_ref(), &stats)
.unwrap()
.is_none());
assert!(r
.push(&packet(final_shard(160, 2, 0)), coder.as_ref(), &stats)
.unwrap()
.is_none());
// A second final claiming 144 bytes (K = 1): geometry-valid alone, but it contradicts the
// pinned totals → dropped.
let before = stats.snapshot().packets_dropped;
assert!(r
.push(&packet(final_shard(144, 1, 0)), coder.as_ref(), &stats)
.unwrap()
.is_none());
assert_eq!(stats.snapshot().packets_dropped, before + 1);
// The frame still completes under the FIRST totals: the rest of block 0 + the final tail.
let mut got = None;
for s in 1..8u16 {
assert!(got.is_none());
got = r
.push(&packet(sentinel_shard(s)), coder.as_ref(), &stats)
.unwrap();
}
assert!(got.is_none(), "block 1 still owes a shard");
let got = r
.push(&packet(final_shard(160, 2, 1)), coder.as_ref(), &stats)
.unwrap()
.expect("frame completes under the first pinned totals");
assert_eq!(got.data.len(), 160);
}