fix(kernel): reject forward compression pointers in DNS name decoding - #183
Merged
Conversation
Audit of `dns.rs` ahead of wiring it into the UDP receive path. The parser was already hardened against the two classic DNS name-decompression hangs: both `decode_domain_name` and `skip_name` cap pointer indirection at `MAX_POINTERS` (16), all offset arithmetic is saturating, label length is bounded at 63, and `rdlength`/output copies are bounds-checked. Pointer cycles and self-pointers are therefore already terminated by the hop cap. What was missing is the RFC 1035 §4.1.4 prior-occurrence rule. A compression pointer names a name that appeared *earlier* in the message, so its target must be strictly before the pointer. `decode_domain_name` only checked `ptr < data.len()`, never `ptr < cur`, so a pointer could point forward at bytes that were never labels and reconstruct an attacker-controlled "name". RFC 9267 calls out exactly this omission, and it is the BIND/tcpdump CVE-2017-12995 fix. Enforce `ptr < cur`, comparing against the position of the pointer just read so chained hops must each move strictly backward. This makes cycles and self-pointers unreachable by construction instead of relying on the hop cap, and rejects forward pointers the cap never counted. `skip_name` returned `cur + 2` for a pointer without checking the second byte exists; a pointer at the final byte yielded an offset one past the message end. Guard it. Five regression tests. Two fail without the fix (`rejects_forward_pointer`, `rejects_truncated_pointer`); the self-pointer, cycle, and legal-backward-chain-over-the-cap cases already passed via the hop cap and are kept as guards so a future relaxation of the backward rule turns them red. `test_decode_domain_name_pointer` (offset 13 → 0, a legitimate backward pointer) still passes, confirming valid packets are unaffected. Suite: 4601 passed, 0 skipped.
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.
Audit of
crates/kernel/src/dns.rsahead of wiring it into the UDP receivepath, per the
crates/kernel/AGENTS.mdrule that an unwired parser must beaudited before it becomes reachable from remote, pre-authentication input.
Audit result
The two classic DNS name-decompression hangs were already prevented: both
decode_domain_nameandskip_namecap pointer indirection atMAX_POINTERS(16), all offset arithmetic is saturating, label length is bounded at 63, and
rdlength/ output copies are bounds-checked. Pointer cycles and self-pointersare terminated by that hop cap. Overflow/underflow and unbounded-loop classes:
clean.
The defect
Missing was the RFC 1035 §4.1.4 prior-occurrence rule. A compression
pointer names a name that appeared earlier in the message, so its target must
be strictly before the pointer.
decode_domain_namechecked onlyptr < data.len()— neverptr < cur— so a pointer could point forwardat bytes that were never labels and reconstruct an attacker-controlled "name".
RFC 9267 explicitly lists this omission as a recurring parser anti-pattern, and
enforcing it is the BIND / tcpdump CVE-2017-12995 fix.
The hop cap does not cover this: a forward pointer consumes a hop but still
succeeds within 16 hops, so the integrity violation slips through even though
the loop terminates.
Fix
decode_domain_name: rejectptr >= cur, comparing against the position ofthe pointer just read so chained hops each move strictly backward. This makes
cycles and self-pointers unreachable by construction rather than relying on
the hop cap, and rejects forward pointers the cap never counted.
skip_name: a pointer at the final byte returnedcur + 2— one past theend of the message — without checking the second byte exists. Guard it.
Tests (TDD: written RED first)
Two fail without the fix:
Three already passed via the hop cap and are kept as guards so a future
relaxation of the backward rule turns them red:
test_decode_domain_name_pointer(a legitimate backward pointer, offset 13 →0) still passes, confirming valid packets are unaffected.
Verification
cargo fmt --all -- --check·cargo clippy --workspace -- -D warnings·cargo build --workspacebash scripts/run-tests.sh— 4601 passed, 0 skipped (kernel 359 → 367)Follow-up
Wiring DNS validation into the UDP port-53 path is a separate PR (one concern
per PR). The parser is now safe to expose.