Skip to content

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

@bluetoothbot

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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions