Skip to content

Commit 9946ef1

Browse files
authored
Various DHCP fixes (#3912)
* Parse zero-length DHCP options as well Until 0db2a8c was merged zero-length DHCP options were parsed at least once and the "options" list contained tuples with at least two elements. This patch restores that. It prevents `repr(DHCP(...))` from crashing with something like ``` TypeError: ord() expected a character, but string of length 23 found ``` and also lets the fields with meaningfull empty values to be initialized properly. For example, ``` >>> DHCP(b"\x79\x00").options [('classless_static_routes',)] ``` turns into ``` [("classless_static_routes", [])] ``` It's a follow-up to 0db2a8c * Fix the "Classless Static Route" parser FieldListField expects its fields to either fully consume bytes or throw exceptions if those bytes can't be consumed. The "Classless Static Route" field didn't do that when it received invalid subnet mask widths and it led to a loop where FieldListField tried to parse the same sequence of bytes and append the same value to the list over and over again until scapy got interrupted manually or got killed by the OOM killer. This patch addresses that by throwing exceptions when classless static routes can't be consumed. It basically relies on inet_ntoa rejecting anything other than 4 bytes. The test suite is updated to cover the corner cases. Other than that the DHCP parser was fuzzed for about an hour and nothing popped up. It's a follow-up to f5b7188
1 parent 9eb6237 commit 9946ef1

File tree

2 files changed

+33
-8
lines changed

2 files changed

+33
-8
lines changed

scapy/layers/dhcp.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,15 +188,7 @@ def i2m(self, pkt, x):
188188
return struct.pack('b', prefix) + dest + router
189189

190190
def getfield(self, pkt, s):
191-
if not s:
192-
return None
193-
194191
prefix = orb(s[0])
195-
# if prefix is invalid value ( 0 > prefix > 32 ) then break
196-
if prefix > 32 or prefix < 0:
197-
warning("Invalid prefix value: %d (0x%x)", prefix, prefix)
198-
return s, []
199-
200192
route_len = 5 + (prefix + 7) // 8
201193
return s[route_len:], self.m2i(pkt, s[:route_len])
202194

@@ -449,6 +441,16 @@ def m2i(self, pkt, x):
449441
else:
450442
olen = orb(x[1])
451443
lval = [f.name]
444+
445+
if olen == 0:
446+
try:
447+
_, val = f.getfield(pkt, b'')
448+
except Exception:
449+
opt.append(x)
450+
break
451+
else:
452+
lval.append(val)
453+
452454
try:
453455
left = x[2:olen + 2]
454456
while left:

test/scapy/layers/dhcp.uts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,29 @@ assert p4[DHCP].options[0] == ("mud-url", b"https://example.org")
8484
p5 = IP(s5)
8585
assert DHCP in p5
8686
assert p5[DHCP].options[0] == ("classless_static_routes", ["192.168.123.4/32:10.0.0.1", "169.254.254.0/24:10.0.1.2"])
87+
88+
repr(DHCP(b"\x01\x00"))
89+
assert DHCP(b"\x01\x00").options == [b"\x01\x00"]
90+
assert DHCP(b"\x28\x00").options == [("NIS_domain", b"")]
91+
assert DHCP(b"\x37\x00").options == [("param_req_list", [])]
92+
assert DHCP(b"\x79\x00").options == [("classless_static_routes", [])]
93+
assert DHCP(b"\x01\x0C\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b").options == [("subnet_mask", "0.1.2.3", "4.5.6.7", "8.9.10.11")]
94+
95+
b = b"\x79\x01\xff"
96+
p = DHCP(b)
97+
assert p.options == [b]
98+
p.clear_cache()
99+
assert raw(p) == b
100+
101+
b = b"\x79\x0a\x21\x01\x02\x03\x04\x05\x06\x07\x08\x09"
102+
p = DHCP(b)
103+
assert p.options == [b]
104+
p.clear_cache()
105+
assert raw(p) == b
106+
107+
b = b"\x79\x09\x20\x01\x02\x03\x04\x05\x06\x07\x08\xff"
108+
assert DHCP(b).options == [("classless_static_routes", ["1.2.3.4/32:5.6.7.8"]), "end"]
109+
87110
= DHCPOptions
88111

89112
# Issue #2786

0 commit comments

Comments
 (0)