Skip to content

Commit f20e851

Browse files
bdracobdraco
committed
Ensure the name cache is rolled back when the packet reaches maximum size
If the packet was too large, it would be rolled back at the end of write_record. We need to remove the names that were added to the name cache (self.names) as well to avoid a case were we would create a pointer to a name that was rolled back. The size of the packet was incorrect at the end after the inserts because insert_short would increase self.size even though it was already accounted before. To resolve this insert_short_at_start was added which does not increase self.size. This did not cause an actual bug, however it sure made debugging this problem far more difficult. Additionally the size now inserted and then replaced when the actual size is known because it made debugging quite difficult since the size did not previously agree with the data.
1 parent 8f7effd commit f20e851

2 files changed

Lines changed: 177 additions & 19 deletions

File tree

‎zeroconf/__init__.py‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,10 +1013,13 @@ def write_byte(self, value: int) -> None:
10131013
"""Writes a single byte to the packet"""
10141014
self.pack(b'!c', int2byte(value))
10151015

1016-
def insert_short(self, index: int, value: int) -> None:
1017-
"""Inserts an unsigned short in a certain position in the packet"""
1018-
self.data.insert(index, struct.pack(b'!H', value))
1019-
self.size += 2
1016+
def insert_short_at_start(self, value: int) -> None:
1017+
"""Inserts an unsigned short at the start of the packet"""
1018+
self.data.insert(0, struct.pack(b'!H', value))
1019+
1020+
def replace_short(self, index: int, value: int) -> None:
1021+
"""Replaces an unsigned short in a certain position in the packet"""
1022+
self.data[index] = struct.pack(b'!H', value)
10201023

10211024
def write_short(self, value: int) -> None:
10221025
"""Writes an unsigned short to the packet"""
@@ -1123,22 +1126,24 @@ def write_record(self, record: DNSRecord, now: float, allow_long: bool = False)
11231126
self.write_int(record.get_remaining_ttl(now))
11241127
index = len(self.data)
11251128

1126-
# Adjust size for the short we will write before this record
1127-
self.size += 2
1129+
self.write_short(0) # Will get replaced with the actual size
11281130
record.write(self)
1129-
self.size -= 2
1130-
1131-
length = sum((len(d) for d in self.data[index:]))
1132-
# Here is the short we adjusted for
1133-
self.insert_short(index, length)
1134-
1131+
# Adjust size for the short we will write before this record
1132+
length = sum((len(d) for d in self.data[index + 1 :]))
1133+
# Here we replace the 0 length short we wrote
1134+
# before with the actual length
1135+
self.replace_short(index, length)
11351136
len_limit = _MAX_MSG_ABSOLUTE if allow_long else _MAX_MSG_TYPICAL
11361137

11371138
# if we go over, then rollback and quit
11381139
if self.size > len_limit:
11391140
while len(self.data) > start_data_length:
11401141
self.data.pop()
11411142
self.size = start_size
1143+
1144+
rollback_names = [name for name, idx in self.names.items() if idx >= start_size]
1145+
for name in rollback_names:
1146+
del self.names[name]
11421147
return False
11431148
return True
11441149

@@ -1207,15 +1212,15 @@ def packets(self) -> List[bytes]:
12071212
if self.write_record(additional, 0):
12081213
additionals_written += 1
12091214

1210-
self.insert_short(0, additionals_written)
1211-
self.insert_short(0, authorities_written)
1212-
self.insert_short(0, answers_written)
1213-
self.insert_short(0, questions_written)
1214-
self.insert_short(0, self.flags)
1215+
self.insert_short_at_start(additionals_written)
1216+
self.insert_short_at_start(authorities_written)
1217+
self.insert_short_at_start(answers_written)
1218+
self.insert_short_at_start(questions_written)
1219+
self.insert_short_at_start(self.flags)
12151220
if self.multicast:
1216-
self.insert_short(0, 0)
1221+
self.insert_short_at_start(0)
12171222
else:
1218-
self.insert_short(0, self.id)
1223+
self.insert_short_at_start(self.id)
12191224
self.packets_data.append(b''.join(self.data))
12201225
self.reset_for_next_packet()
12211226

‎zeroconf/test.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,3 +1973,156 @@ def send(out, addr=r._MDNS_ADDR, port=r._MDNS_PORT):
19731973

19741974
# unregister
19751975
zc.unregister_service(info)
1976+
1977+
1978+
def test_dns_compression_rollback_for_corruption():
1979+
"""Verify rolling back does not lead to dns compression corruption."""
1980+
out = r.DNSOutgoing(r._FLAGS_QR_RESPONSE | r._FLAGS_AA)
1981+
address = socket.inet_pton(socket.AF_INET, "192.168.208.5")
1982+
1983+
additionals = [
1984+
{
1985+
"name": "HASS Bridge ZJWH FF5137._hap._tcp.local.",
1986+
"address": address,
1987+
"port": 51832,
1988+
"text": b"\x13md=HASS Bridge"
1989+
b" ZJWH\x06pv=1.0\x14id=01:6B:30:FF:51:37\x05c#=12\x04s#=1\x04ff=0\x04"
1990+
b"ci=2\x04sf=0\x0bsh=L0m/aQ==",
1991+
},
1992+
{
1993+
"name": "HASS Bridge 3K9A C2582A._hap._tcp.local.",
1994+
"address": address,
1995+
"port": 51834,
1996+
"text": b"\x13md=HASS Bridge"
1997+
b" 3K9A\x06pv=1.0\x14id=E2:AA:5B:C2:58:2A\x05c#=12\x04s#=1\x04ff=0\x04"
1998+
b"ci=2\x04sf=0\x0bsh=b2CnzQ==",
1999+
},
2000+
{
2001+
"name": "Master Bed TV CEDB27._hap._tcp.local.",
2002+
"address": address,
2003+
"port": 51830,
2004+
"text": b"\x10md=Master Bed"
2005+
b" TV\x06pv=1.0\x14id=9E:B7:44:CE:DB:27\x05c#=18\x04s#=1\x04ff=0\x05"
2006+
b"ci=31\x04sf=0\x0bsh=CVj1kw==",
2007+
},
2008+
{
2009+
"name": "Living Room TV 921B77._hap._tcp.local.",
2010+
"address": address,
2011+
"port": 51833,
2012+
"text": b"\x11md=Living Room"
2013+
b" TV\x06pv=1.0\x14id=11:61:E7:92:1B:77\x05c#=17\x04s#=1\x04ff=0\x05"
2014+
b"ci=31\x04sf=0\x0bsh=qU77SQ==",
2015+
},
2016+
{
2017+
"name": "HASS Bridge ZC8X FF413D._hap._tcp.local.",
2018+
"address": address,
2019+
"port": 51829,
2020+
"text": b"\x13md=HASS Bridge"
2021+
b" ZC8X\x06pv=1.0\x14id=96:14:45:FF:41:3D\x05c#=12\x04s#=1\x04ff=0\x04"
2022+
b"ci=2\x04sf=0\x0bsh=b0QZlg==",
2023+
},
2024+
{
2025+
"name": "HASS Bridge WLTF 4BE61F._hap._tcp.local.",
2026+
"address": address,
2027+
"port": 51837,
2028+
"text": b"\x13md=HASS Bridge"
2029+
b" WLTF\x06pv=1.0\x14id=E0:E7:98:4B:E6:1F\x04c#=2\x04s#=1\x04ff=0\x04"
2030+
b"ci=2\x04sf=0\x0bsh=ahAISA==",
2031+
},
2032+
{
2033+
"name": "FrontdoorCamera 8941D1._hap._tcp.local.",
2034+
"address": address,
2035+
"port": 54898,
2036+
"text": b"\x12md=FrontdoorCamera\x06pv=1.0\x14id=9F:B7:DC:89:41:D1\x04c#=2\x04"
2037+
b"s#=1\x04ff=0\x04ci=2\x04sf=0\x0bsh=0+MXmA==",
2038+
},
2039+
{
2040+
"name": "HASS Bridge W9DN 5B5CC5._hap._tcp.local.",
2041+
"address": address,
2042+
"port": 51836,
2043+
"text": b"\x13md=HASS Bridge"
2044+
b" W9DN\x06pv=1.0\x14id=11:8E:DB:5B:5C:C5\x05c#=12\x04s#=1\x04ff=0\x04"
2045+
b"ci=2\x04sf=0\x0bsh=6fLM5A==",
2046+
},
2047+
{
2048+
"name": "HASS Bridge Y9OO EFF0A7._hap._tcp.local.",
2049+
"address": address,
2050+
"port": 51838,
2051+
"text": b"\x13md=HASS Bridge"
2052+
b" Y9OO\x06pv=1.0\x14id=D3:FE:98:EF:F0:A7\x04c#=2\x04s#=1\x04ff=0\x04"
2053+
b"ci=2\x04sf=0\x0bsh=u3bdfw==",
2054+
},
2055+
{
2056+
"name": "Snooze Room TV 6B89B0._hap._tcp.local.",
2057+
"address": address,
2058+
"port": 51835,
2059+
"text": b"\x11md=Snooze Room"
2060+
b" TV\x06pv=1.0\x14id=5F:D5:70:6B:89:B0\x05c#=17\x04s#=1\x04ff=0\x05"
2061+
b"ci=31\x04sf=0\x0bsh=xNTqsg==",
2062+
},
2063+
{
2064+
"name": "AlexanderHomeAssistant 74651D._hap._tcp.local.",
2065+
"address": address,
2066+
"port": 54811,
2067+
"text": b"\x19md=AlexanderHomeAssistant\x06pv=1.0\x14id=59:8A:0B:74:65:1D\x05"
2068+
b"c#=14\x04s#=1\x04ff=0\x04ci=2\x04sf=0\x0bsh=ccZLPA==",
2069+
},
2070+
{
2071+
"name": "HASS Bridge OS95 39C053._hap._tcp.local.",
2072+
"address": address,
2073+
"port": 51831,
2074+
"text": b"\x13md=HASS Bridge"
2075+
b" OS95\x06pv=1.0\x14id=7E:8C:E6:39:C0:53\x05c#=12\x04s#=1\x04ff=0\x04ci=2"
2076+
b"\x04sf=0\x0bsh=Xfe5LQ==",
2077+
},
2078+
]
2079+
2080+
out.add_answer_at_time(
2081+
DNSText(
2082+
"HASS Bridge W9DN 5B5CC5._hap._tcp.local.",
2083+
r._TYPE_TXT,
2084+
r._CLASS_IN | r._CLASS_UNIQUE,
2085+
r._DNS_OTHER_TTL,
2086+
b'\x13md=HASS Bridge W9DN\x06pv=1.0\x14id=11:8E:DB:5B:5C:C5\x05c#=12\x04s#=1'
2087+
b'\x04ff=0\x04ci=2\x04sf=0\x0bsh=6fLM5A==',
2088+
),
2089+
0,
2090+
)
2091+
2092+
for record in additionals:
2093+
out.add_additional_answer(
2094+
r.DNSService(
2095+
record["name"], # type: ignore
2096+
r._TYPE_SRV,
2097+
r._CLASS_IN | r._CLASS_UNIQUE,
2098+
r._DNS_HOST_TTL,
2099+
0,
2100+
0,
2101+
record["port"], # type: ignore
2102+
record["name"], # type: ignore
2103+
)
2104+
)
2105+
out.add_additional_answer(
2106+
r.DNSText(
2107+
record["name"], # type: ignore
2108+
r._TYPE_TXT,
2109+
r._CLASS_IN | r._CLASS_UNIQUE,
2110+
r._DNS_OTHER_TTL,
2111+
record["text"], # type: ignore
2112+
)
2113+
)
2114+
out.add_additional_answer(
2115+
r.DNSAddress(
2116+
record["name"], # type: ignore
2117+
r._TYPE_A,
2118+
r._CLASS_IN | r._CLASS_UNIQUE,
2119+
r._DNS_HOST_TTL,
2120+
record["address"], # type: ignore
2121+
)
2122+
)
2123+
2124+
for packet in out.packets():
2125+
# Verify we can process the packets we created to
2126+
# ensure there is no corruption with the dns compression
2127+
incoming = r.DNSIncoming(packet)
2128+
assert incoming.valid is True

0 commit comments

Comments
 (0)