fix(mesh): bound mDNS TXT reassembly allocation (HIGH — unauth packet aborts engine, inv #1/#10) - #236
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the panic!-in-closure with a recorded Cell flag asserted false, per the 3-panel guardrails review. The hostile-count reassembly tests now fail as a clean assertion (not a panic) if the pre-allocation bound ever regresses and a chunk key is looked up. Equivalent strength; no test weakened. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
Live end-to-end runtime validation (rule 26 — real path, hostile input)Beyond the unit tests, the lane owner drove the real public browse path (
|
aperim-agent
added a commit
that referenced
this pull request
Jul 10, 2026
…#53) (#243) The #236 alloc-bound fix caps mDNS TXT reassembly at MAX_CHUNKS=64 (12.8 KiB at CHUNK_BYTES=200) so an unauthenticated packet cannot drive an unbounded allocation (inv #1/#10). This adds the residual headroom check: a maximal *legitimate* AnnouncePayload must reassemble well under that cap, so the guard rejects only oversized/malicious input, never real traffic — closing the false-negative robustness gap the Codex panel flagged on #236. The only variable-length announce fields are `digests` and the fixed 64-byte Ed25519 signature. The digest set mirrors the machine fingerprint, which models exactly five ComponentKinds (board/cpu/nic/disk/gpu) scored one-per-kind, so a canonical announce carries ~5 salted digests. The test over-provisions by an order of magnitude — 64 distinct digests, every byte 0xFF for a strict worst-case JSON width (serde_json encodes each [u8; 32] digest and the signature as an array of decimal integers) — and asserts, via the real `to_wire()` encoder and the real `reassemble_txt` guard, that it lands at 44/64 chunks (20 free, 31% headroom) and losslessly round-trips. A real ~5-digest host is ~6 chunks. In-crate unit test: the `pub(crate)` cap constants and `reassemble_txt` are `cfg(any(feature = "mdns", test))`, so it runs in the default `cargo test -p multiview-mesh` gate with no feature or socket. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
aperim-agent
added a commit
that referenced
this pull request
Jul 11, 2026
…bservably (#94) (#252) * test(mesh): RED — publish-side MAX_CHUNKS guard refuses an over-cap announce (#94) The mDNS publish path (service.rs `chunk_properties` + `MeshTransport::announce`) emits `c=chunks.len()` + `p0..pN` unconditionally — the <=MAX_CHUNKS=64 bound exists only receive-side (transport.rs `reassemble_txt`). A payload that ever exceeded 64 chunks would be emitted, then silently dropped by every peer's receive-side bound, so this node would vanish from the mesh with no observable failure at the source. Add two failing publish-side tests (mirroring the receive-side cap tests) that exercise a not-yet-existent pure helper `chunk_txt` + a typed `MeshError::AnnounceTooLarge`: - chunk_txt_refuses_a_wire_past_the_chunk_cap: MAX_CHUNKS+1 chunks -> typed error - chunk_txt_accepts_the_cap_and_round_trips: at-cap publishes + round-trips Fails to compile (chunk_txt + AnnounceTooLarge absent) — the RED baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mesh): refuse a publish-side announce over MAX_CHUNKS, observably (#94) The mDNS publish path emitted `c=chunks.len()` + `p0..pN` unconditionally — the <=MAX_CHUNKS=64 bound existed only receive-side (transport.rs `reassemble_txt`). An announce that ever exceeded 64 chunks would be emitted, then silently dropped by every peer's receive-side bound, so the node would vanish from the mesh with no observable failure at the source (the publish-side half of the Codex 3-panel residual on the #236 alloc-bound fix; task #94). Add a publish-side guard symmetric with the receive side: - transport.rs: pure `chunk_txt(wire) -> Result<Vec<(String,String)>, MeshError>` under cfg(any(feature="mdns", test)), so the regression runs in the DEFAULT `cargo test -p multiview-mesh` gate, not just under --features mdns. It caps chunks at MAX_CHUNKS and returns a typed error rather than emit an over-cap announce. Deliberate asymmetry vs. `reassemble_txt`: the receive side silently ignores untrusted over-cap input (None); the publish side is our OWN payload, so an over-cap announce is a real, log-worthy fault (Result). - error.rs: new `MeshError::AnnounceTooLarge { chunks, max }` (non-breaking — the enum is #[non_exhaustive]). - service.rs: `MdnsService::announce` now calls `chunk_txt(wire)?` (the duplicated chunker is removed); an over-cap payload is never emitted. - driver.rs: `announce_browse_step` logs AnnounceTooLarge at warn! (a recurring fault that removes the node from discovery); transient Transport errors stay debug!. Never a panic (invariant #10 — best-effort/isolation). Defence in depth: today's payload shape (~5 salted digests) keeps a legitimate announce far under the cap (44/64 worst-case, #53); the guard makes a future over-cap growth observable at the source instead of a silent discovery failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(mesh): RED — chunk_txt must refuse a non-UTF-8 chunk, not silently drop it (#94) A chunk that is not valid UTF-8 cannot ride an mDNS TXT string value. The current chunk_txt sets c = chunks.len() but silently skips such a chunk (`if let Ok(text) = from_utf8`), so it could return Ok with c=N and only N-1 `p` properties — every peer's reassemble_txt then misses `p{index}` and drops the whole announce silently. That is the exact silent-drop this guard exists to prevent, living inside the guard (rule 27: the guarding comment is aspirational). This RED asserts chunk_txt refuses a non-UTF-8 chunk with a typed `MeshError::AnnounceNotText { chunk_index }` (index 1, to pin the real index). It fails to compile until the variant exists — the same RED shape accepted for the AnnounceTooLarge guard in this PR. Cross-vendor (Codex) review finding on PR #252. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(mesh): refuse a non-UTF-8 announce chunk, never silently drop it (#94) chunk_txt set c = chunks.len() but silently skipped any chunk that failed from_utf8 (`if let Ok(text) = …`). A skipped chunk emits c=N with only N-1 `p` properties, so every peer's reassemble_txt misses `p{index}`, its `?` short-circuits, and the whole announce is silently dropped — the exact silent-drop this guard exists to prevent, hiding inside the guard behind an aspirational "cannot fail here" comment (rule 27). Make chunk_txt's contract TOTAL: a non-UTF-8 chunk is refused with a typed MeshError::AnnounceNotText { chunk_index }, so it returns either Ok with one `p{i}` per chunk (props.len() == chunks.len() + 1) or a typed Err — never Ok with a chunk omitted. The driver logs it at warn (parity with AnnounceTooLarge): a discoverability-fatal fault is observable, not silent. Today's announce is pure-ASCII JSON (integer/hex arrays, kebab-case enums, RFC3339 instants — no free-form string field), so this never fires in practice; the guard is defence in depth against future payload growth. Comments in transport.rs / service.rs now state the enforced invariant, not "impossible". - error.rs: new #[non_exhaustive] variant AnnounceNotText { chunk_index } (non-breaking; cli/control/licence still cargo check clean). - transport.rs: from_utf8(chunk).map_err(AnnounceNotText)?; + # Errors docs. - service.rs: module doc + announce comment state the total contract. - driver.rs: AnnounceNotText → warn! (not the best-effort debug! arm). Closes the Codex cross-vendor finding on PR #252. Gate: fmt; clippy -p multiview-mesh --all-targets -D warnings (default + mdns); test -p multiview-mesh (default 9 lib + mdns, mdns_live #[ignore]d). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Summary
An unauthenticated single link-local mDNS packet with a large TXT chunk-count (
cproperty) droveVec::with_capacity(count * 200)to a ~200 TB allocation →handle_alloc_error→ whole-process abort → the multiview engine goes off air. This violates invariant #1 ("never off air") and #10 ("a best-effort mesh input is physically incapable of killing the engine"). Active in all three shipped deploy presets. (CWE-789/1284/400.)Found by an adversarial read-only mesh re-audit that two prior audit passes missed.
Fix
reassemble_txt()helper (transport.rs) and cap the attacker-controlled count atMAX_CHUNKS = 64before any allocation, multiplication, or property lookup;checked_mulcloses integer overflow.service.rs) delegates to the bounded helper.Tests (TDD, RED→GREEN)
test(mesh): reject unbounded mDNS reassembly counts(hostilec = 1_000_000_000_000rejected before reading chunks; exact-max and one-over-max boundaries).mdns),cargo test -p multiview-mesh(default +mdns),cargo check --workspace.Review
Cross-vendor 3-reviewer Codex panel (invariant #1/#10, security): NO DEFECT ×3 (security, correctness/regression, guardrails). Residual (silent-reject if a legit
AnnouncePayloadever exceeds 64×200 B) tracked as a low follow-up.🤖 Generated with Claude Code