Skip to content

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

Closed

Description

@JarryShaw

Each IP-in-IP layer dissects the next one recursively, so deep nesting exhausts the stack. beholder (pcapkit/utilities/decorators.py:128) catches the RecursionError, logs it and leaves the rest Raw, with no warning. This is the tunnel counterpart of #1604. #1608 bounds extension-header chains only, and its count restarts at every tunnel.

Repro on main (c43e9318c), Python 3.14. The capture is Ethernet, then n IPv4 headers each carrying protocol 4, then IPv4/TCP carrying data. The default engine reassembles 1 TCP datagram for n ≤ 105 and 0 for n ≥ 108, with the protocol chain ending at IPv4. #1608's head behaves the same.

The #1608 review also reports two related failures, which I have not re-measured:

  • IPv6-in-IPv6 fails at 100 levels.
  • Three 30-header extension chains joined by IPv4-in-IPv4 lose TCP at about 87 headers.

Decision needed: how to bound it.

  1. Recommended: one per-packet bound on nested internet-layer headers, counting extension headers and encapsulated IP together. On reaching it, stop, leave the rest Raw and warn once, which is the shape of the fix(protocols): a chain of ~80+ IPv6 extension headers exhausts recursion and leaves the payload undissected #1604 ruling. Counting per packet rather than per chain also removes the reset gap.
  2. A separate bound for tunnel nesting only, keeping fix(protocols): bound the IPv6 extension-header chain at 32 headers (#1604) #1608's per-chain count.
  3. Dissect iteratively instead of recursively, so no bound is needed. This is a larger change, centred on ProtocolBase._parse_next_layer.

Any of these edits #1608's files, so the work waits for #1608 to merge.

Activity

  1. added
    fixPull requests that fix a defect (fix: subject prefix)
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 10, 2026
  2. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Widened after the question on #1604: this is not tunnel-specific. On #1608's head, Python 3.14, 200 stacked 802.1Q tags stop at layer 109 (TCP 0, no warning), while 1, 50 and 100 tags reach TCP. Any protocol that hands its payload to the next layer recurses through ProtocolBase._parse_next_layer.

    Recommendation:

    Go with that, and with 64?

  3. changed the title [-]fix(protocols): deeply nested IP-in-IP tunnels exhaust the recursion limit and lose TCP silently[/-] [+]fix(protocols): deep layer nesting of any protocol exhausts the recursion limit and loses TCP silently[/+] on Oct 10, 2026
  4. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    I'm considering to bound the recursion limit at top level so recursive encapsulation will be properly handled globally.

  5. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: the owner leans towards one top-level bound, so that recursive encapsulation is handled the same way for every protocol. That matches the recommendation above. Two specifics to confirm before a worker starts:

    1. A counted layer bound, not a raised interpreter limit. The bound would count layers in ProtocolBase._parse_next_layer and stop at a fixed number. Raising sys.setrecursionlimit instead would be process-wide, so it would change the caller's own program. A deep enough frame would still crash the C stack. And its cut-off would vary with the caller's stack depth, so dpkt, scapy and pyshark could not match it.
    2. 64 layers per frame. On reaching it: stop, leave the rest Raw, warn once. fix(protocols): bound the IPv6 extension-header chain at 32 headers (#1604) #1608's 32-header IPv6 chain bound stays.

    The work edits files that #1608 also edits, so it starts once #1608 merges.

  6. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Yea sounds good.

  7. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    And mark this bound as a design decision that may need future revisit in the PEP. We may consider moving to an iteration model in future releases. But for now, recursion with depth limit is the way forward.

  8. JarryShaw commented on Oct 10, 2026

    @JarryShaw
    OwnerAuthor

    Recorded: the owner accepted both specifics.

    • The bound is a counted one: 64 layers per frame, enforced in ProtocolBase._parse_next_layer. On reaching it, dissection stops, the rest stays Raw, and one warning is raised.
    • fix(protocols): bound the IPv6 extension-header chain at 32 headers (#1604) #1608's 32-header IPv6 chain bound stays.
    • The owner also asked that the PEP record this as a design decision open to revisit. Recursion with a depth limit is the approach for now, and an iterative model may replace it in a later release. That goes in a new section of docs/source/contributing/pep.rst, in the same PR.

    A worker is on the default engine now. Its work is stacked on #1608, because both edit the same files, so its PR opens once #1608 merges. pypcapfile, dpkt, scapy and pyshark follow in #1609, which now covers both bounds.

  9. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Oct 10, 2026
  10. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 12, 2026
  11. added this to the PyPCAPKit v1.5 milestone on Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

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

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions