Skip to content

fix(protocols): bound a frame's dissection at 64 layers (#1610) - #1620

Merged
JarryShaw merged 1 commit into
mainfrom
fix/frame-layer-depth-bound
Oct 12, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/frame-layer-depth-bound

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Closes #1610 for the default engine, on top of #1608. dpkt, scapy, pyshark and pypcapfile follow in #1609.

  • Cause. Each layer constructs the next inside its own parse (protocol.py:2323, about 9 to 12 frames a layer), so deep nesting of any protocol hit the recursion limit. beholder (decorators.py:128) only logged the RecursionError and left the rest Raw.
  • Fix. FRAME_LAYER_LIMIT = 64 in pcapkit/protocols/protocol.py, counted per frame in ProtocolBase._parse_next_layer. Past it, the rest stays Raw, with one ProtocolWarning that names the bound. Tunnels do not restart the count. IPv6 counts the extension headers it walks, since they are protocol-chain entries too. A walked ESP still dissects its decrypted payload, and those layers count on after the headers walked before it. sys.setrecursionlimit is not touched.
  • Every protocol's rebuild path changes. ProtocolBase._make_payload is the step of every from_data that rebuilds the payload. It now raises the layer count by one around that nested from_data. Each rebuilt layer parses its own octets again, and before this it counted from zero. So from_data of a cut frame dissected 64 more layers below every layer, which went past the recursion limit under pytest (depth 966 against 721 for the parse). Each of those re-parses also warned of the bound, 65 warnings for one frame. The payload's re-parse no longer warns, so a rebuilt cut frame warns once, as its parse does. fix(protocols): bound the IPv6 extension-header chain at 32 headers (#1604) #1608's chain bound still warns 2 to 33 times on a rebuild; that is fix(protocols): rebuilding a frame cut at the extension-chain bound warns more than once #1619.
  • Why the rebuild is byte-identical. The call, its arguments and the octets each layer packs are unchanged. The count only decides where the re-parse stops, and the Raw it leaves keeps every octet. Below 64 layers the bound is never reached, so nothing differs. At 65 and 1,000 layers, from_data(info.to_dict()).data equals the capture, and the rebuilt protocol chain equals the parsed one. All 201 files under tests/protocols/ pass on 3.11 and 3.14, run one at a time.
  • Headroom. Under pytest at the default limit, the costliest 64-layer frame needs 834 frames on 3.11, leaving 166 spare, and 756 on 3.14. That frame is IPv4 carrying a chain of 32 extension headers, then IPv4 in IPv4 carrying 28 more, then TCP. IP in IP and 802.1Q need 636 to 776.
  • Toolkit. With TCP exactly the 65th layer, the default engine still reassembled and traced it. The Raw the bound leaves carries an error, as a TCP header the parser rejected does, and the IP above it carries TCP. So tcp_segment's fix(engines): TCP trace flows disagree between engines on snapped frames and on IPv4 total_length=0 #1518 fall-back (toolkit/pcap.py:157, reached through fix(toolkit): key tunnelled TCP by the innermost IP header (#1581) #1605's innermost-IP walk) read it back out. The bound now marks that Raw with ProtocolBase._past_layer_limit, and the fall-back declines it. A rejected header below the bound is read as before. fix(protocols): bound the IPv6 extension-header chain at 32 headers (#1604) #1608's extension-header bound cannot leak this way, because the layer it cuts is always an extension header, never TCP, so its error convention is unchanged.
  • Docs. The bound is on the protocol base page. The PEP records recursion with a depth limit as a decision open to revisit.

- Each layer dissects the next from inside its own parse, so deep nesting of
  any protocol, such as IP in IP or stacked 802.1Q tags, hit the recursion
  limit, and beholder left the rest raw, silently; #1604's bound restarts at
  every tunnel. FRAME_LAYER_LIMIT (64) now caps the layers
  ProtocolBase._parse_next_layer dissects in one frame: past it the rest
  stays Raw, with one ProtocolWarning.
- IPv6 counts the extension headers it walks as layers, as the protocol
  chain does, and stops its walk at the bound; what a walked ESP decrypts
  counts on after them.
- _make_payload builds each payload one layer further in, so from_data of a
  cut frame re-parses no deeper than the parse did, where it went past the
  recursion limit before, and warns of the bound once, not once a layer.
- tcp_segment read a TCP 65th layer back out of the Raw the bound leaves, as
  it does a header the parser rejected (#1518); that Raw is now marked
  _past_layer_limit, and is not read.
- The PEP records recursion with a depth limit as a decision open to revisit.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at b0d32247c.

Carried over from the Sonnet cross-review, whose final round found no defect at 5763209ed (then stacked on #1608's head). Since #1608 merged, the commit is re-applied on 9f37fb051 as the same patch (git patch-id --stable 3150484d both sides), so the verdict carries.

Checked on the rebased tree, since #1613 and #1614 also landed: the #1610 test file and ten toolkit and engine files (dpkt, scapy, pyshark, pypcapfile, #1581, #1604, #1607, engine agreement) all pass on 3.11 and 3.14, run one at a time.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.72% (unit tier, Python 3.14, b0d32247c, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2884 147 1010 36 94.07%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17152 226 4578 191 98.06%
pcapkit/toolkit 931 134 334 6 84.35%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 01151b1 into main Oct 12, 2026
38 checks passed
@JarryShaw
JarryShaw deleted the fix/frame-layer-depth-bound branch October 12, 2026 23:36
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 12, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 12, 2026
JarryShaw added a commit that referenced this pull request Oct 13, 2026
…arns once (#1619) (#1630)

- Each layer from_data rebuilds parses its own octets again, and a layer
  constructed rather than parsed started at _exthdr_depth 0, so each rebuilt
  extension header counted its chain afresh: its parse went up to 32 headers
  past the cut, and warned again on reaching EXTENSION_HEADER_LIMIT. A cut
  frame rebuilt with 2 warnings over IPv6 and up to 33 over IPv4, not 1.
- Internet._make_payload now hands the payload its place in the chain, one
  past this layer's, as #1620 hands it its place in the frame; a constructed
  IPv6_Ext takes it up, so the rebuild stops where the parse did.
- _next_layer_class does not warn of the bound inside a payload being
  rebuilt, as _parse_next_layer does not of FRAME_LAYER_LIMIT: the layer
  being rebuilt warns once, from its own parse.
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(protocols): deep layer nesting of any protocol exhausts the recursion limit and loses TCP silently

1 participant