Skip to content

sessions: bound missing ranges in TCP reassembly - #5112

Merged
gpotter2 merged 2 commits into
secdev:masterfrom
KernelClint:fix/tcp-sequence-gap-bound
Aug 27, 2026
Merged

sessions: bound missing ranges in TCP reassembly#5112
gpotter2 merged 2 commits into
secdev:masterfrom
KernelClint:fix/tcp-sequence-gap-bound

Conversation

@KernelClint

Copy link
Copy Markdown
Contributor

TCPSession stores stream data at offsets derived from TCP sequence numbers. One far-ahead byte
causes StringBuffer.append() to allocate a zero-filled gap at
scapy/sessions.py:105-124,
then every retransmission materializes and checks the attacker-sized buffer at
scapy/sessions.py:354-364.

From 128 to 1,024 updates, median capture processing grew from 14.69 ms to 551.49 ms, with a 1.91
exponent and a 4.0% noise floor. Patched times were 7.10 ms to 56.05 ms, with a 1.00 exponent and a
4.8% noise floor. Valid contiguous append changed by -1.3%, within a 3.0% noise floor.

This change ignores missing ranges larger than Scapy's 65,535-byte receive limit. The focused
boundary regression failed on the unmodified revision and passed with the patch.

AI-Assisted: yes (GPT-5.6-Cyber)
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.09%. Comparing base (ba8641a) to head (c84ee6f).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5112      +/-   ##
==========================================
- Coverage   80.59%   80.09%   -0.51%     
==========================================
  Files         390      390              
  Lines       96892    96905      +13     
==========================================
- Hits        78094    77612     -482     
- Misses      18798    19293     +495     
Files with missing lines Coverage Δ
scapy/sessions.py 93.93% <100.00%> (+0.24%) ⬆️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread scapy/sessions.py Outdated
Adds max_gap=MTU to the constructor and warns when data is dropped.

AI-Assisted: yes (GPT-5.6-Cyber)
@KernelClint

Copy link
Copy Markdown
Contributor Author

Both done in c84ee6f1.

StringBuffer.__init__ now takes max_gap=MTU, and both drop sites log
"Dropped data further than allowed per 'max_gap'." The warning is a real improvement — the old
behaviour discarded data silently, which is the part that made this hard to notice.

Added two tests: one that a custom max_gap is honoured in both directions, one that a larger
max_gap allows a gap the default would reject. Both fail if the guard is removed.

@gpotter2
gpotter2 merged commit 23808d9 into secdev:master Aug 27, 2026
21 of 23 checks passed
@gpotter2

Copy link
Copy Markdown
Member

Thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants