Repository navigation
fix(toolkit): scapy reads nothing in a zero-length frame, and a hand-built packet off its octets (#1676) - #1683
Conversation
…built packet off its octets (#1676) Scapy dissects a packet only from a non-empty string, so a zero-length frame comes back as a layer of defaults: at RAW and IPV4 an IPv4 header whose ihl and len are None, and bytes() of it builds a 20-octet header from nothing. - _frame: the octets of a packet's frame -- packet.original for one read from a capture (which carries a wirelen), else bytes(packet). - _network: no layer for a frame with no octets, so no adapter hands over an input for it and IPv4 reassembly no longer raises TypeError. - packet2dict, packet2frame: report _frame(), so a zero-length frame carries b'' rather than a built header, and a hand-built packet's frame record carries the octets it builds rather than none. - ipv4_reassembly, _tcp_segment, _within_datagrams: read the IHL, Total Length and Data Offset off the octets, which Scapy leaves None on a hand-built layer, so IPv4 and TCP reassembly and TCP flow tracing no longer raise on one. New tests/toolkit/test_scapy_zero_length_frames_1676_unit.py: 5 tests, all failing on main and passing with the fix on 3.14 and 3.11.
|
Verdict: GOOD TO GO at Root cause. Scapy dissects only a non-empty string. A zero-length frame therefore comes back as a layer of defaults: an IPv4 layer whose The fix. It tells a frame read from a capture from a hand-built packet by What the reviewer measured. 376 of its own cases, each a zero-length, truncated or malformed frame followed by six controls, across 9 link types and both scapy releases. On |
|
Coverage: 89.44% (unit tier, Python 3.14,
Per-file detail: the |
Third-party engines are best effort (#1687), so the scapy code that exists only to match the default engine comes out: - revert #1683, #1663, #1652 and #1631, and the scapy hunks of #1614: the link-type and loopback-family walk, the frame and extension-header bounds, the IPv6 chain walk over the octets, the stops at GRE, VXLAN, PPPoE and the other layers the default engine leaves raw and at ICMP quotes, and the IPv4 header checks - keep #1624's walk (TCP behind AH or an IPv6 extension header in IPv4, and the later-fragment stop), #1605's keying by the carrying IP header and #1589's trailer cut; the walk calls _walk_extension_headers, which the pypcapfile revert keeps - re-land as guards: a zero-length frame is no IPv4 input and has no packet2dict octets (#1676); IPv4 in IPv6 ends at the Payload Length (#1623); a packet built by hand is read off its octets (#1676); ipv6_reassembly takes only its own IPv6 layer's Fragment header, and none in a later IPv4 fragment - delete the four scapy agreement modules and test_scapy_unit's default-engine checks; the guards' tests assert constants - docs: scapy applies the extension-header bound only behind IPv4 and the frame-layer bound nowhere, and its silent gaps are listed where gaps are said to be announced The scapy toolkit and engine modules, the shared modules that run scapy, and the project and tier guards pass on 3.14 (scapy 2.7.0) and 3.11 (scapy 2.8.0).
Third-party engines are best effort (#1687), so the scapy code that exists only to match the default engine comes out: - revert #1683, #1663, #1652 and #1631, and the scapy hunks of #1614: the link-type and loopback-family walk, the frame and extension-header bounds, the IPv6 chain walk over the octets, the stops at GRE, VXLAN, PPPoE and the other layers the default engine leaves raw and at ICMP quotes, and the IPv4 header checks - keep #1624's walk (TCP behind AH or an IPv6 extension header in IPv4, and the later-fragment stop), #1605's keying by the carrying IP header and #1589's trailer cut; the walk calls _walk_extension_headers, which the pypcapfile revert keeps - re-land as guards: a zero-length frame is no IPv4 input and has no packet2dict octets (#1676); IPv4 in IPv6 ends at the Payload Length (#1623); a packet built by hand is read off its octets (#1676); ipv6_reassembly takes only its own IPv6 layer's Fragment header, and none in a later IPv4 fragment - delete the four scapy agreement modules and test_scapy_unit's default-engine checks; the guards' tests assert constants - docs: scapy applies the extension-header bound only behind IPv4 and the frame-layer bound nowhere, and its silent gaps are listed where gaps are said to be announced The scapy toolkit and engine modules, the shared modules that run scapy, and the project and tier guards pass on 3.14 (scapy 2.7.0) and 3.11 (scapy 2.8.0).
Please follow the guide below
You will be asked some questions, please read them carefully and answer honestly
Put an
xinto all the boxes [ ] relevant to your pull request (like that [x])Use Preview tab to see how your pull request will actually look like
Searched for similar pull requests
Followed the coding style (
make pylint,make mypy,make isort)make testpasses, and a test case covers the change — tests cover it; only the scapy, toolkit, engine and integration parity selections were run, not the whole suiteAdded a changelog entry under
docs/source/changelog/and regeneratedCHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657What is the purpose of your pull request?
Tick the commit type your subject line carries.
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingrelease— bumps the version or rolls up a distributionchore— anything elseDescription of your pull request and other information
Root cause: Scapy dissects only a non-empty string, so a zero-length frame comes back as a layer of defaults. At RAW and IPV4 that is an IPv4 header whose
ihlandlenareNone, andbytes()builds 20 octets for it out of nothing. A packet built by hand has the sameNonefields._frame(): a packet's frame octets. That ispacket.originalfor a packet read from a capture (which has awirelen), elsebytes(packet)._networkreturns no layer for an empty frame.packet2dictandpacket2framenow reportb''for one, and a hand-built packet's frame record carries its built octets, notb''.ipv4_reassembly,_tcp_segmentand_within_datagramsread the IHL, Total Length and Data Offset from the octets. That stops theTypeErroron hand-built layers.Measured on 3.14.8 (scapy 2.7.0) and 3.11.15 (scapy 2.8.0), with the same results on both:
main, scapy raised on the zero-length RAW and IPV4 frames and agreed on every other case. With the fix, all 104 agree. dpkt agrees on all 104 before and after.bytes(packet): 19 adapter calls raisedTypeErroronmain, and all now match. The one exception is tracing an IP-rooted packet, which still raisesMissingKeyError(Registries mint a permanent member for every unrecognised value: 1,169 extend_enum sites across 113 registries #775, by design).original == bytes(packet)for every one, sopacket2dictoutput does not change.New
tests/toolkit/test_scapy_zero_length_frames_1676_unit.pyhas 5 tests. All 5 fail onmainand pass with the fix, on both interpreters. Reverting any one of the 7 changes makes at least one test fail. Coverage ofscapy.pygoes from 99.58% to 99.59%.Closes #1676