Skip to content

Security: _read_string/_read_character_string skip remaining-byte validation, enabling silent record-data corruption #1752

Description

@bluetoothbot

Problem

_read_character_string (line 253) reads length = self.view[self.offset] (1 byte) and then slices self.data[self.offset : self.offset + length]. _read_string(length) (line 261) is called with an attacker-controlled 16-bit rdlength field (line 282) and slices the same way. Python's slice silently returns fewer bytes when the end index is past the buffer, but self.offset += length still advances by the declared length — past self._data_len. The constructed DNSText/DNSAddress/DNSHinfo record is appended to self._answers (line 301) with the silently-truncated payload before any exception is raised. The follow-up _read_name() for the next record fails because offset is now out-of-buffer, terminating the parse, but the malformed record has already entered the answer list and from there the cache and ServiceInfo. An attacker on the LAN can stuff a TXT record with declared rdlength=65535 and 10 actual bytes — consumers calling ServiceInfo.properties parse 10 bytes worth of key=value pairs while the wire said 65535. Same is true for a 4-byte _read_string(4) (A) or _read_string(16) (AAAA) when the packet is truncated: ZeroconfIPv4Address/IPv6Address construction will eventually see a wrong-length bytes.

Why This Matters

Cache-poisoning surface in a protocol that already has no source authentication. The realistic impact is corruption of TXT properties / address records surfaced to downstream consumers (Home Assistant, etc.), where the consumer trusts that the decoded record matches the wire. It is not RCE, but it is a parser-state-desync class bug — the kind of bug that is the building block for higher-impact chains.

Suggested Fix

Reject early when length exceeds the remaining buffer rather than relying on Python's slice tolerance and downstream _read_name to fail:

def _read_string(self, length: _int) -> bytes:
    end = self.offset + length
    if end > self._data_len:
        raise IncomingDecodeError(
            f"rdata length {length} at offset {self.offset} overruns packet end "
            f"(data_len={self._data_len}) from {self.source}"
        )
    info = self.data[self.offset : end]
    self.offset = end
    return info

Mirror the same check in _read_character_string, including a guard that self.offset < self._data_len before the length byte is read. The per-record try/except in _read_others already restores self.offset = end for the next record, so the only effect is that truncated/over-advertised payloads are skipped instead of silently retained.

Details

Severity 🟡 Medium
Category memory_safety
Location src/zeroconf/_protocol/incoming.py:253-265
Effort ⚡ Quick fix

🤖 Created by Kōan from audit session

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions