fix(packet): read_qname doesn't reject label length > 63, swallows malformed upstream packets #142

Open
opened 2026-04-23 22:19:03 +08:00 by razvandimescu · 0 comments
razvandimescu commented 2026-04-23 22:19:03 +08:00 (Migrated from github.com)

Summary

buffer::read_qname doesn't validate that a label-length byte is ≤ 63 (RFC 1035 §2.3.1). If an upstream emits a malformed compression pointer landing inside a label body (rather than at a label-length boundary, violating RFC 1035 §4.1.4), read_qname reads an arbitrary byte as the length and consumes up to 63+ bytes of garbage as a single "label". write_qname then rejects the result, ctx::resolve_query hits the generic error path, and the client gets a TC-flagged empty response.

Found during #137 field check against Tailscale MagicDNS (100.100.100.100:53).

Reproduction

[upstream]
mode = "forward"
address = ["100.100.100.100:53"]   # Tailscale MagicDNS
$ kdig @127.0.0.1 odin.adobe.com HTTPS
;; WARNING: truncated reply from 127.0.0.1@53(UDP), retrying over TCP
...

MagicDNS responds with a mostly-uncompressed 219-byte packet whose SOA mname pointer c0 5a lands at offset 90, which falls inside the adobeaemcloud label body — not at a label-length boundary.

Why it matters

Current failure mode is silent: TC fallback with empty answers, clients fall back to TCP or just fail. The internal log line ("response too large") is also misleading — the buffer wasn't full, write_qname just rejected a bogus label.

Orthogonal to #137: on main, opaque-UNKNOWN re-emits the bad bytes and clients see "malformed reply packet" (the original #128 symptom). Post-#137, the SOA is parsed natively so the bad pointer is followed and the break shifts to the write side. Numa should refuse to propagate upstream malformedness either way.

Suggested fix

  1. src/buffer.rs::read_qname: after reading a length byte with len & 0xC0 == 0x00, require len <= 63. Return a parse error otherwise. The 5-jump pointer limit stays.
  2. src/ctx.rs: distinguish "wire buffer full" (genuine TC) from "parse/serialize error" (upstream malformed → SERVFAIL + fail over), instead of collapsing both into TC.

Test plan

  • Unit: handcrafted packet where label length is 0x61 → read_qname returns Err.
  • Unit: handcrafted packet where compression pointer lands mid-label → read_qname returns Err.
  • Integration: mock upstream emits MagicDNS-style malformed SOA → Numa returns SERVFAIL or fails over, not empty TC.
## Summary `buffer::read_qname` doesn't validate that a label-length byte is ≤ 63 (RFC 1035 §2.3.1). If an upstream emits a malformed compression pointer landing inside a label body (rather than at a label-length boundary, violating RFC 1035 §4.1.4), `read_qname` reads an arbitrary byte as the length and consumes up to 63+ bytes of garbage as a single "label". `write_qname` then rejects the result, `ctx::resolve_query` hits the generic error path, and the client gets a TC-flagged empty response. Found during #137 field check against Tailscale MagicDNS (`100.100.100.100:53`). ## Reproduction ```toml [upstream] mode = "forward" address = ["100.100.100.100:53"] # Tailscale MagicDNS ``` ``` $ kdig @127.0.0.1 odin.adobe.com HTTPS ;; WARNING: truncated reply from 127.0.0.1@53(UDP), retrying over TCP ... ``` MagicDNS responds with a mostly-uncompressed 219-byte packet whose SOA mname pointer `c0 5a` lands at offset 90, which falls inside the `adobeaemcloud` label body — not at a label-length boundary. ## Why it matters Current failure mode is silent: TC fallback with empty answers, clients fall back to TCP or just fail. The internal log line ("response too large") is also misleading — the buffer wasn't full, write_qname just rejected a bogus label. Orthogonal to #137: on `main`, opaque-UNKNOWN re-emits the bad bytes and clients see "malformed reply packet" (the original #128 symptom). Post-#137, the SOA is parsed natively so the bad pointer is followed and the break shifts to the write side. Numa should refuse to propagate upstream malformedness either way. ## Suggested fix 1. `src/buffer.rs::read_qname`: after reading a length byte with `len & 0xC0 == 0x00`, require `len <= 63`. Return a parse error otherwise. The 5-jump pointer limit stays. 2. `src/ctx.rs`: distinguish "wire buffer full" (genuine TC) from "parse/serialize error" (upstream malformed → SERVFAIL + fail over), instead of collapsing both into TC. ## Test plan - Unit: handcrafted packet where label length is 0x61 → `read_qname` returns `Err`. - Unit: handcrafted packet where compression pointer lands mid-label → `read_qname` returns `Err`. - Integration: mock upstream emits MagicDNS-style malformed SOA → Numa returns SERVFAIL or fails over, not empty TC.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dearsky/numa#142