fix: bound DNS compression-pointer chain depth in DNSIncoming by bdraco · Pull Request #1719 · python-zeroconf/python-zeroconf

bdraco · 2026-05-18T02:37:19Z

Summary

Closes #1713. A crafted mDNS packet chaining thousands of forward DNS
name-compression pointers (RFC 1035 §4.1.4) into a single ~3 kB datagram
blows DNSIncoming._decode_labels_at_offset through CPython's default
recursion limit (sys.getrecursionlimit() == 1000). RecursionError
was not in DECODE_EXCEPTIONS, so it escaped DNSIncoming.__init__
and bubbled up to asyncio's default exception handler — turning a single
small multicast packet from any host on the local link (a guest on the
same Wi-Fi, a compromised IoT device, a container on a shared bridge)
into sustained CPU burn and debug-log flooding. Home Assistant on
Raspberry-Pi-class hardware is the canonical victim. Adds a hard cap
on the pointer-chain depth, mirroring the existing label cap.

Details

seen_pointers already blocked cycles and MAX_DNS_LABELS = 128
already capped the number of labels, but nothing capped the chain
length of unique forward pointers. A _MAX_MSG_ABSOLUTE (8966 B)
packet can carry ~4000 2-byte pointers, each pointing to the next
unique offset.
_decode_labels_at_offset gains a depth: unsigned int parameter
(declared in _protocol/incoming.pxd). Top-of-function check raises
IncomingDecodeError when depth > MAX_DNS_LABELS — the same
bound as the existing label cap, so no new constant. The single
external caller (_read_name) passes 0; the recursive call site
passes depth + 1.
Choice of bound: MAX_DNS_LABELS (128) is plenty for any legitimate
name — a 253-byte name (the RFC 6762 §16 / RFC 1035 §3.1 limit) caps
out at ~127 single-byte labels even with every label minimized. The
recursion depth is bounded by the number of pointer follows, which
in well-formed packets is ≤ the label count, so legitimate traffic
never approaches the cap.
Belt-and-braces: RecursionError is added to DECODE_EXCEPTIONS
so any future regression that lets the recursion run away is contained
as an invalid packet rather than logged by asyncio's default handler.
Per-source-IP rate limiting is out of scope here — this PR closes the
recursion-DoS class on its own; quotas would be a separate change to
the listener.
CWE-674 (Uncontrolled Recursion). LAN-local attack surface only.

Cython perf check

cython -a on the touched _protocol/incoming.py shows every new
line as score-0 (no Python interpreter overhead):

line 412:  self._decode_labels_at_offset(original_offset, labels, seen_pointers, 0)  (score-0)
line 425:  if depth > MAX_DNS_LABELS:                                                  (score-0)
line 466:  self._decode_labels_at_offset(link, linked_labels, seen_pointers, depth + 1)  (score-0)

Generated C compiles depth > MAX_DNS_LABELS to a direct
unsigned int > unsigned int compare, depth + 1 to a C add,
and the recursive call to the existing C-level vtable dispatch with
one extra unsigned int argument. Function entry score is unchanged
(score-11, same boilerplate). Wall-clock smoke test on a typical
compressed mDNS response (best-of-10 over 50k iters, REQUIRE_CYTHON=1):

PREFIX  best: 1237.2 ns/parse
POSTFIX best: 1234.1 ns/parse

Within noise — the per-call overhead is one C compare at function
entry and one C add per pointer follow.

Test plan

SKIP_CYTHON=1 poetry run pytest tests/test_protocol.py —
47 passed (46 pre-existing + 1 new regression below). Coverage
report shows 98 % on _protocol/incoming.py with the 5
uncovered lines (143, 199–200, 210, 213) all pre-existing and
unrelated to this PR — every line touched by this diff is hit.
REQUIRE_CYTHON=1 poetry install --only=main,dev rebuilds
the extension against the new .pxd; full suite
poetry run pytest --timeout=60 — 338 passed, 3 skipped.
poetry run pre-commit run --files <changed files> — all
hooks pass (ruff lint + format, mypy, flake8, codespell,
cython-lint, pyupgrade).
New regression: tests/test_protocol.py::test_dns_compression_pointer_chain_depth_attack
feeds a 1500-pointer forward chain through DNSIncoming and
asserts valid is False with no exception escaping. Verified
to fail on pre-fix master with
RecursionError: maximum recursion depth exceeded.
(optional) draft a GitHub Security Advisory so a CVE can be
assigned alongside the patched release.

bdraco · 2026-05-18T02:38:10Z

@bluetoothbot review

codspeed-hq · 2026-05-18T02:39:43Z

Merging this PR will not alter performance

✅ 6 untouched benchmarks

_{Comparing fix/dns-compression-pointer-chain-depth (2e07f25) with master (64d143d)}

codecov · 2026-05-18T02:40:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.76%. Comparing base (64d143d) to head (2e07f25).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1719   +/-   ##
=======================================
  Coverage   99.76%   99.76%           
=======================================
  Files          33       33           
  Lines        3426     3428    +2     
  Branches      471      472    +1     
=======================================
+ Hits         3418     3420    +2     
  Misses          5        5           
  Partials        3        3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR hardens DNSIncoming against a denial-of-service vector where deeply chained DNS compression pointers can trigger excessive recursion (and uncaught RecursionError) during name decoding.

Changes:

Add an explicit depth counter to _decode_labels_at_offset to bound compression-pointer chain depth and raise IncomingDecodeError when exceeded.
Add RecursionError to DECODE_EXCEPTIONS to ensure unexpected recursion failures are contained as “invalid packet” handling.
Add a regression test that constructs a 1500-pointer forward-chain packet and asserts the parse is marked invalid without exceptions escaping.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
`tests/test_protocol.py`	Adds regression coverage for a deep compression-pointer chain attack scenario.
`src/zeroconf/_protocol/incoming.py`	Implements pointer-chain depth limiting and broadens decode exception handling to include `RecursionError`.
`src/zeroconf/_protocol/incoming.pxd`	Updates the Cython signature to match the new `_decode_labels_at_offset(..., depth)` parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Closes #1713. A crafted mDNS packet can chain thousands of forward compression pointers into a single ~3 kB datagram. ``DNSIncoming._decode_labels_at_offset`` recurses once per pointer it follows (RFC 1035 §4.1.4 name compression), so a deep chain blows through CPython's default recursion limit (``sys.getrecursionlimit() == 1000``). ``RecursionError`` was *not* in ``DECODE_EXCEPTIONS``, so it escaped ``DNSIncoming.__init__`` and bubbled up to asyncio's default exception handler — turning a single small multicast packet from any host on the local link (a guest on the same Wi-Fi, a compromised IoT device, a container on a shared bridge) into sustained CPU burn (each crash unwinds ~1000 frames + the asyncio exception machinery) and debug-log flooding. Home Assistant deployments on Raspberry-Pi-class hardware are the canonical victim. ``seen_pointers`` already blocked cycles and ``MAX_DNS_LABELS = 128`` already capped the *number of labels*, but nothing capped the *chain length of unique forward pointers*. A ``_MAX_MSG_ABSOLUTE`` (8966 B) packet can carry ~4000 2-byte pointers, each pointing to the next. Thread an explicit ``depth`` counter through ``_decode_labels_at_offset`` and raise ``IncomingDecodeError`` when it exceeds ``MAX_DNS_LABELS`` — same bound as the existing label cap, so no new constant. Belt-and- braces, add ``RecursionError`` to ``DECODE_EXCEPTIONS`` so any future regression is contained as an invalid packet rather than logged by asyncio's default handler. ``incoming.pxd`` updated in the same commit so the Cython build picks up the new ``unsigned int depth`` parameter. ``cython -a`` confirms the depth check and ``depth + 1`` recursive-call argument both compile to score-0 (pure C) — direct ``unsigned int`` compare and a C add — with no Python interaction added to the hot path. Function entry score is unchanged (score-11, same boilerplate). Wall-clock smoke test on a typical compressed mDNS response (best-of-10 over 50k iters, REQUIRE_CYTHON=1): 1237.2 ns/parse pre-fix vs 1234.1 ns/parse post-fix — within noise. CWE-674 (Uncontrolled Recursion). LAN-local attack surface only.

bluetoothbot · 2026-05-18T02:46:04Z

PR Review — fix: bound DNS compression-pointer chain depth in DNSIncoming

Clean, well-bounded security fix for CWE-674. The depth cap reuses MAX_DNS_LABELS (128) — a principled choice because legitimate names are bounded by MAX_NAME_LENGTH=253 bytes, which caps the label count well below 128, and pointer-follow depth in well-formed packets is ≤ label count. The check fires at function entry (line 425), so failed parses don't pollute _name_cache (line 461 never executes on raise). The .pxd signature in incoming.pxd:86 is mirrored correctly with unsigned int depth, matching the existing TTL/offset/length convention and avoiding the sign-compare class of bug the Cython gotchas section in CLAUDE.md warns about. Adding RecursionError to DECODE_EXCEPTIONS is sound belt-and-braces — a future regression that drops the depth arg or adds a new recursive site stays contained as an invalid packet rather than escaping to asyncio's default handler. The regression test constructs a deterministic 1500-pointer forward chain and asserts observable behavior (valid is False, questions == []) without coupling to the exception type, which is the right pattern. CodSpeed confirms no perf regression on the Cython hot path. Ship it.

Checklist

Cython .pxd signature mirrors .py change
Depth bound justified by RFC limits
No partial state cached on failed recursion
Regression test covers the attack scenario
Test asserts observable behavior, not implementation
Performance neutral on Cython build
Defense-in-depth via DECODE_EXCEPTIONS
Free-threading safe (no new module-level mutable state)

Summary

Clean, well-bounded security fix for CWE-674. The depth cap reuses MAX_DNS_LABELS (128) — a principled choice because legitimate names are bounded by MAX_NAME_LENGTH=253 bytes, which caps the label count well below 128, and pointer-follow depth in well-formed packets is ≤ label count. The check fires at function entry (line 425), so failed parses don't pollute _name_cache (line 461 never executes on raise). The .pxd signature in incoming.pxd:86 is mirrored correctly with unsigned int depth, matching the existing TTL/offset/length convention and avoiding the sign-compare class of bug the Cython gotchas section in CLAUDE.md warns about. Adding RecursionError to DECODE_EXCEPTIONS is sound belt-and-braces — a future regression that drops the depth arg or adds a new recursive site stays contained as an invalid packet rather than escaping to asyncio's default handler. The regression test constructs a deterministic 1500-pointer forward chain and asserts observable behavior (valid is False, questions == []) without coupling to the exception type, which is the right pattern. CodSpeed confirms no perf regression on the Cython hot path. Ship it.

Automated review by Kōan2e07f25

bdraco requested a review from Copilot May 18, 2026 02:38

Copilot started reviewing on behalf of bdraco May 18, 2026 02:39 View session

Copilot AI reviewed May 18, 2026

View reviewed changes

bdraco had a problem deploying to release May 18, 2026 02:43 — with GitHub Actions Error

bdraco force-pushed the fix/dns-compression-pointer-chain-depth branch from b35bea9 to 2e07f25 Compare May 18, 2026 02:43

bdraco temporarily deployed to release May 18, 2026 02:47 — with GitHub Actions Inactive

bdraco merged commit f9e2359 into master May 18, 2026
35 checks passed

bdraco deleted the fix/dns-compression-pointer-chain-depth branch May 18, 2026 02:48

This was referenced May 29, 2026

Extend Local Audio Out provider with PulseAudio support music-assistant/server#3724

Open

⬆️ Update music-assistant-frontend to 2.17.169 music-assistant/server#4031

Merged

Bump stages on 2.9 release music-assistant/server#3942

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: bound DNS compression-pointer chain depth in DNSIncoming#1719

fix: bound DNS compression-pointer chain depth in DNSIncoming#1719
bdraco merged 1 commit into
masterfrom
fix/dns-compression-pointer-chain-depth

bdraco commented May 18, 2026 •

edited

Loading

Uh oh!

bdraco commented May 18, 2026

Uh oh!

codspeed-hq Bot commented May 18, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

bluetoothbot commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bdraco commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Cython perf check

Test plan

Uh oh!

bdraco commented May 18, 2026

Uh oh!

codspeed-hq Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

codecov Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bluetoothbot commented May 18, 2026

PR Review — fix: bound DNS compression-pointer chain depth in DNSIncoming

Checklist

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bdraco commented May 18, 2026 •

edited

Loading

codspeed-hq Bot commented May 18, 2026 •

edited

Loading

codecov Bot commented May 18, 2026 •

edited

Loading