fix: bound NSEC bitmap length against record end (#1731)

bluetoothbot · bdraco · web-flow · commit 1d83550c58ed · 2026-05-19T16:54:19.000-05:00
Co-authored-by: J. Nick Koston &lt;nick+github@koston.org&gt;
Co-authored-by: J. Nick Koston &lt;nick@home-assistant.io&gt;
diff --git a/src/zeroconf/_protocol/incoming.pxd b/src/zeroconf/_protocol/incoming.pxd
@@ -128,6 +128,7 @@ cdef class DNSIncoming:
         byte="unsigned int",
         i="unsigned int",
         bitmap_length="unsigned int",
+        bitmap_end="unsigned int",
     )
     cdef list _read_bitmap(self, unsigned int end)
 
diff --git a/src/zeroconf/_protocol/incoming.py b/src/zeroconf/_protocol/incoming.py
@@ -388,9 +388,23 @@ def _read_bitmap(self, end: _int) -> list[int]:
             offset = self.offset
             offset_plus_one = offset + 1
             offset_plus_two = offset + 2
+            # RFC 4034 §4.1.2: each window block is window-number byte +
+            # bitmap-length byte (1..32) + bitmap. A bitmap_length that walks
+            # past the record's declared end would otherwise leave self.offset
+            # pointing inside (or past) the next record header, corrupting
+            # every subsequent record in the same packet.
+            if offset_plus_two > end:
+                raise IncomingDecodeError(
+                    f"NSEC bitmap window header truncated at offset {offset} from {self.source}"
+                )
             window = view[offset]
             bitmap_length = view[offset_plus_one]
             bitmap_end = offset_plus_two + bitmap_length
+            if bitmap_length == 0 or bitmap_length > 32 or bitmap_end > end:
+                raise IncomingDecodeError(
+                    f"NSEC bitmap length {bitmap_length} invalid or overruns record end "
+                    f"at offset {offset} from {self.source}"
+                )
             for i, byte in enumerate(self.data[offset_plus_two:bitmap_end]):
                 for bit in range(8):
                     if byte & (0x80 >> bit):
diff --git a/tests/test_protocol.py b/tests/test_protocol.py
@@ -807,6 +807,85 @@ def test_parse_packet_with_nsec_record():
     assert nsec_record.next_name == "MyHome54 (2)._meshcop._udp.local."
 
 
+def test_nsec_bitmap_length_overruns_record_end():
+    """Reject NSEC bitmap whose declared length runs past the record boundary."""
+    # 0 questions, 2 answers. Answer 1 is a malformed NSEC: rdlength=9, but the
+    # bitmap window claims length=255 — overrunning the record. Answer 2 is a
+    # PTR that must still parse because the offset for the next record stays
+    # pinned to the NSEC's declared end.
+    packet = (
+        b"\x00\x00\x84\x00\x00\x00\x00\x02\x00\x00\x00\x00"
+        b"\x04test\x05local\x00"
+        b"\x00\x2f\x80\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x09"
+        b"\xc0\x0c"
+        b"\x00\xff"
+        b"\x80\x00\x00\x00\x00"
+        b"\xc0\x0c"
+        b"\x00\x0c\x00\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x02"
+        b"\xc0\x0c"
+    )
+    parsed = r.DNSIncoming(packet)
+    answers = parsed.answers()
+    ptrs = [a for a in answers if isinstance(a, r.DNSPointer)]
+    assert len(ptrs) == 1
+    assert ptrs[0].alias == "test.local."
+    # The malformed NSEC must not surface — if it did, it would carry rdtypes
+    # synthesized from bytes past the record boundary.
+    assert not any(isinstance(a, r.DNSNsec) for a in answers)
+
+
+def test_nsec_bitmap_zero_length_window_rejected():
+    """A bitmap window with length=0 violates RFC 4034 §4.1.2 and must be rejected."""
+    packet = (
+        b"\x00\x00\x84\x00\x00\x00\x00\x02\x00\x00\x00\x00"
+        b"\x04test\x05local\x00"
+        b"\x00\x2f\x80\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x04"
+        b"\xc0\x0c"
+        b"\x00\x00"
+        b"\xc0\x0c"
+        b"\x00\x0c\x00\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x02"
+        b"\xc0\x0c"
+    )
+    parsed = r.DNSIncoming(packet)
+    answers = parsed.answers()
+    ptrs = [a for a in answers if isinstance(a, r.DNSPointer)]
+    assert len(ptrs) == 1
+    assert not any(isinstance(a, r.DNSNsec) for a in answers)
+
+
+def test_nsec_bitmap_truncated_window_header_rejected():
+    """Reject NSEC bitmap with a trailing byte too short to hold a window header."""
+    packet = (
+        b"\x00\x00\x84\x00\x00\x00\x00\x02\x00\x00\x00\x00"
+        b"\x04test\x05local\x00"
+        b"\x00\x2f\x80\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x06"
+        b"\xc0\x0c"
+        b"\x00\x01\x80"
+        b"\xff"
+        b"\xc0\x0c"
+        b"\x00\x0c\x00\x01"
+        b"\x00\x00\x11\x94"
+        b"\x00\x02"
+        b"\xc0\x0c"
+    )
+    parsed = r.DNSIncoming(packet)
+    answers = parsed.answers()
+    ptrs = [a for a in answers if isinstance(a, r.DNSPointer)]
+    assert len(ptrs) == 1
+    assert ptrs[0].alias == "test.local."
+    assert not any(isinstance(a, r.DNSNsec) for a in answers)
+
+
 def test_records_same_packet_share_fate():
     """Test records in the same packet all have the same created time."""
     out = r.DNSOutgoing(const._FLAGS_QR_QUERY | const._FLAGS_AA)

Original file line number	Diff line number	Diff line change
`@@ -128,6 +128,7 @@ cdef class DNSIncoming:`
`128`	`128`	`byte="unsigned int",`
`129`	`129`	`i="unsigned int",`
`130`	`130`	`bitmap_length="unsigned int",`
	`131`	`+ bitmap_end="unsigned int",`
`131`	`132`	`)`
`132`	`133`	`cdef list _read_bitmap(self, unsigned int end)`
`133`	`134`