Skip to content

Commit 0e5e637

Browse files
bluetoothbotbluetoothbot
authored
fix: bound QuestionHistory size to prevent LAN-driven OOM (#1733)
1 parent 69bbcd1 commit 0e5e637

4 files changed

Lines changed: 85 additions & 2 deletions

File tree

‎src/zeroconf/_history.pxd‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@ from ._dns cimport DNSQuestion
44

55

66
cdef cython.double _DUPLICATE_QUESTION_INTERVAL
7+
cdef unsigned int _MAX_QUESTION_HISTORY_ENTRIES
78

89
cdef class QuestionHistory:
910

10-
cdef cython.dict _history
11+
cdef public cython.dict _history
1112

1213
cpdef void add_question_at_time(self, DNSQuestion question, double now, cython.set known_answers)
1314

15+
@cython.locals(oldest=DNSQuestion, oldest_entry=cython.tuple, oldest_than=double)
16+
cdef void _evict_to_make_room(self, double now)
17+
1418
@cython.locals(than=double, previous_question=cython.tuple, previous_known_answers=cython.set)
1519
cpdef bint suppresses(self, DNSQuestion question, double now, cython.set known_answers)
1620

‎src/zeroconf/_history.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from __future__ import annotations
2424

2525
from ._dns import DNSQuestion, DNSRecord
26-
from .const import _DUPLICATE_QUESTION_INTERVAL
26+
from .const import _DUPLICATE_QUESTION_INTERVAL, _MAX_QUESTION_HISTORY_ENTRIES
2727

2828
# The QuestionHistory is used to implement Duplicate Question Suppression
2929
# https://datatracker.ietf.org/doc/html/rfc6762#section-7.3
@@ -40,6 +40,8 @@ def __init__(self) -> None:
4040

4141
def add_question_at_time(self, question: DNSQuestion, now: _float, known_answers: set[DNSRecord]) -> None:
4242
"""Remember a question with known answers."""
43+
if question not in self._history and len(self._history) >= _MAX_QUESTION_HISTORY_ENTRIES:
44+
self._evict_to_make_room(now)
4345
self._history[question] = (now, known_answers)
4446

4547
def suppresses(self, question: DNSQuestion, now: _float, known_answers: set[DNSRecord]) -> bool:
@@ -75,3 +77,19 @@ def async_expire(self, now: _float) -> None:
7577
def clear(self) -> None:
7678
"""Clear the history."""
7779
self._history.clear()
80+
81+
def _evict_to_make_room(self, now: _float) -> None:
82+
"""Drop expired or oldest entries when the history is at cap.
83+
84+
Peeks at the oldest insertion (dict is ordered) — only runs the
85+
full O(n) async_expire sweep if it could actually reclaim
86+
something, else a sustained flood at cap turns each insert into
87+
a wasted scan. Falls back to oldest-first eviction.
88+
"""
89+
oldest = next(iter(self._history))
90+
oldest_entry = self._history[oldest]
91+
oldest_than = oldest_entry[0]
92+
if now - oldest_than > _DUPLICATE_QUESTION_INTERVAL:
93+
self.async_expire(now)
94+
while len(self._history) >= _MAX_QUESTION_HISTORY_ENTRIES:
95+
del self._history[next(iter(self._history))]

‎src/zeroconf/const.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@
6565
# to retain by multicasting many unique-name records.
6666
_MAX_CACHE_RECORDS = 10000
6767

68+
# Upper bound on the number of entries QuestionHistory will hold between
69+
# the periodic 10s cache-cleanup ticks. Bounds the memory a malicious LAN
70+
# peer can force the duplicate-question-suppression history to retain by
71+
# flooding distinct questions (RFC 6762 §7.3, defense-in-depth).
72+
_MAX_QUESTION_HISTORY_ENTRIES = 10000
73+
6874
_DNS_PACKET_HEADER_LEN = 12
6975

7076
_MAX_MSG_TYPICAL = 1460 # unused

‎tests/test_history.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,58 @@ def test_question_expire():
7878

7979
# Verify the question not longer suppressed since the cache has expired
8080
assert not history.suppresses(question, now, other_known_answers)
81+
82+
83+
def test_question_history_bounded():
84+
"""History keeps a hard cap so a LAN flood cannot grow it without bound."""
85+
history = QuestionHistory()
86+
now = r.current_time_millis()
87+
answers: set[r.DNSRecord] = set()
88+
89+
cap = const._MAX_QUESTION_HISTORY_ENTRIES
90+
for i in range(cap + 500):
91+
q = r.DNSQuestion(f"_svc{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
92+
history.add_question_at_time(q, now, answers)
93+
94+
assert len(history._history) <= cap
95+
96+
97+
def test_question_history_evicts_oldest_first():
98+
"""When at cap, the oldest insertion is dropped first."""
99+
history = QuestionHistory()
100+
now = r.current_time_millis()
101+
answers: set[r.DNSRecord] = set()
102+
103+
cap = const._MAX_QUESTION_HISTORY_ENTRIES
104+
first = r.DNSQuestion("_first._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
105+
history.add_question_at_time(first, now, answers)
106+
107+
# Add `cap` more fresh, non-expired entries — one past the cap — so the
108+
# final insertion forces oldest-first eviction of `first`.
109+
for i in range(cap):
110+
q = r.DNSQuestion(f"_svc{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
111+
history.add_question_at_time(q, now, answers)
112+
113+
assert first not in history._history
114+
assert len(history._history) <= cap
115+
116+
117+
def test_question_history_opportunistic_expire():
118+
"""Adding past the cap first drops expired entries before evicting fresh ones."""
119+
history = QuestionHistory()
120+
old = r.current_time_millis()
121+
answers: set[r.DNSRecord] = set()
122+
123+
cap = const._MAX_QUESTION_HISTORY_ENTRIES
124+
for i in range(cap):
125+
q = r.DNSQuestion(f"_stale{i}._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
126+
history.add_question_at_time(q, old, answers)
127+
128+
# All prior entries are now stale (>999ms old). Adding one more should
129+
# trigger opportunistic expiry rather than evicting only the oldest one.
130+
fresh_now = old + const._DUPLICATE_QUESTION_INTERVAL + 1
131+
fresh = r.DNSQuestion("_fresh._tcp.local.", const._TYPE_PTR, const._CLASS_IN)
132+
history.add_question_at_time(fresh, fresh_now, answers)
133+
134+
assert fresh in history._history
135+
assert len(history._history) == 1

0 commit comments

Comments
 (0)