fix: bound QuestionHistory size to prevent unbounded LAN-driven growth

bluetoothbot · bluetoothbot · commit 1998e4317296 · 2026-05-18T07:40:56.000Z
QuestionHistory._history was unbounded between the 10s cache-cleanup ticks. A LAN peer streaming distinct questions could grow the dict (and the retained known_answers sets) for ~10s before async_expire ran, easily reaching hundreds of MB at line-rate. Cap _history at _MAX_QUESTION_HISTORY_ENTRIES (10000). When the cap is hit on insert, first call async_expire(now) opportunistically to drop entries past _DUPLICATE_QUESTION_INTERVAL; if the dict is still full, evict the oldest insertion (dict is ordered) until there is room. Match the existing DNSCache cap pattern. Fixes #1723
diff --git a/src/zeroconf/_history.pxd b/src/zeroconf/_history.pxd
@@ -4,6 +4,7 @@ from ._dns cimport DNSQuestion
 
 
 cdef cython.double _DUPLICATE_QUESTION_INTERVAL
+cdef unsigned int _MAX_QUESTION_HISTORY_ENTRIES
 
 cdef class QuestionHistory:
 
diff --git a/src/zeroconf/_history.py b/src/zeroconf/_history.py
@@ -23,7 +23,7 @@
 from __future__ import annotations
 
 from ._dns import DNSQuestion, DNSRecord
-from .const import _DUPLICATE_QUESTION_INTERVAL
+from .const import _DUPLICATE_QUESTION_INTERVAL, _MAX_QUESTION_HISTORY_ENTRIES
 
 # The QuestionHistory is used to implement Duplicate Question Suppression
 # https://datatracker.ietf.org/doc/html/rfc6762#section-7.3
@@ -40,6 +40,14 @@ def __init__(self) -> None:
 
     def add_question_at_time(self, question: DNSQuestion, now: _float, known_answers: set[DNSRecord]) -> None:
         """Remember a question with known answers."""
+        # Bound history size between the periodic 10s cleanup ticks. When at
+        # cap, first drop entries past the duplicate-suppression window; if
+        # still at cap, evict the oldest insertion (dict is ordered).
+        if question not in self._history and len(self._history) >= _MAX_QUESTION_HISTORY_ENTRIES:
+            self.async_expire(now)
+            while len(self._history) >= _MAX_QUESTION_HISTORY_ENTRIES:
+                oldest = next(iter(self._history))
+                del self._history[oldest]
         self._history[question] = (now, known_answers)
 
     def suppresses(self, question: DNSQuestion, now: _float, known_answers: set[DNSRecord]) -> bool:
diff --git a/src/zeroconf/const.py b/src/zeroconf/const.py
@@ -65,6 +65,12 @@
 # to retain by multicasting many unique-name records.
 _MAX_CACHE_RECORDS = 10000
 
+# Upper bound on the number of entries QuestionHistory will hold between
+# the periodic 10s cache-cleanup ticks. Bounds the memory a malicious LAN
+# peer can force the duplicate-question-suppression history to retain by
+# flooding distinct questions (RFC 6762 §7.3, defense-in-depth).
+_MAX_QUESTION_HISTORY_ENTRIES = 10000
+
 _DNS_PACKET_HEADER_LEN = 12
 
 _MAX_MSG_TYPICAL = 1460  # unused
diff --git a/tests/test_history.py b/tests/test_history.py
@@ -78,3 +78,57 @@ def test_question_expire():
 
     # Verify the question not longer suppressed since the cache has expired
     assert not history.suppresses(question, now, other_known_answers)
+
+
+def test_question_history_bounded():
+    """History keeps a hard cap so a LAN flood cannot grow it without bound."""
+    history = QuestionHistory()
+    now = r.current_time_millis()
+    answers: set[r.DNSRecord] = set()
+
+    cap = const._MAX_QUESTION_HISTORY_ENTRIES
+    for i in range(cap + 500):
+        q = r.DNSQuestion(f"_svc{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
+        history.add_question_at_time(q, now, answers)
+
+    assert len(history._history) <= cap
+
+
+def test_question_history_evicts_oldest_first():
+    """When at cap, the oldest insertion is dropped first."""
+    history = QuestionHistory()
+    now = r.current_time_millis()
+    answers: set[r.DNSRecord] = set()
+
+    cap = const._MAX_QUESTION_HISTORY_ENTRIES
+    first = r.DNSQuestion("_first._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
+    history.add_question_at_time(first, now, answers)
+
+    # Fill remaining slots with fresh, non-expired entries.
+    for i in range(cap):
+        q = r.DNSQuestion(f"_svc{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
+        history.add_question_at_time(q, now, answers)
+
+    assert first not in history._history
+    assert len(history._history) <= cap
+
+
+def test_question_history_opportunistic_expire():
+    """Adding past the cap first drops expired entries before evicting fresh ones."""
+    history = QuestionHistory()
+    old = r.current_time_millis()
+    answers: set[r.DNSRecord] = set()
+
+    cap = const._MAX_QUESTION_HISTORY_ENTRIES
+    for i in range(cap):
+        q = r.DNSQuestion(f"_stale{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
+        history.add_question_at_time(q, old, answers)
+
+    # All prior entries are now stale (>999ms old). Adding one more should
+    # trigger opportunistic expiry rather than evicting only the oldest one.
+    fresh_now = old + const._DUPLICATE_QUESTION_INTERVAL + 1
+    fresh = r.DNSQuestion("_fresh._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
+    history.add_question_at_time(fresh, fresh_now, answers)
+
+    assert fresh in history._history
+    assert len(history._history) == 1

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@ from ._dns cimport DNSQuestion`
`4`	`4`
`5`	`5`
`6`	`6`	`cdef cython.double _DUPLICATE_QUESTION_INTERVAL`
	`7`	`+cdef unsigned int _MAX_QUESTION_HISTORY_ENTRIES`
`7`	`8`
`8`	`9`	`cdef class QuestionHistory:`
`9`	`10`