scapy icon indicating copy to clipboard operation
scapy copied to clipboard

Fixed fragment data size in IPv6 defragmentation

Open shapaz opened this issue 1 year ago • 4 comments

Checklist:

  • [ ] If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
  • [ ] I squashed commits belonging together
  • [ ] I added unit tests or explained why they are not relevant
  • [ ] I executed the regression tests (using cd test && ./run_tests or tox)
  • [ ] If the PR is still not finished, please create a Draft Pull Request

fixes #xxx

shapaz avatar Jan 24 '24 10:01 shapaz

Could you share an example that triggers the issue that your are aiming to fix? That will allow us to understand your PR.

guedou avatar Jan 24 '24 10:01 guedou

Codecov Report

Merging #4228 (6c901ba) into master (d71014a) will decrease coverage by 32.67%. Report is 3 commits behind head on master. The diff coverage is 0.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4228       +/-   ##
===========================================
- Coverage   81.77%   49.10%   -32.67%     
===========================================
  Files         331      331               
  Lines       76721    76725        +4     
===========================================
- Hits        62736    37675    -25061     
- Misses      13985    39050    +25065     
Files Coverage Δ
scapy/layers/inet6.py 41.04% <0.00%> (-47.50%) :arrow_down:

... and 234 files with indirect coverage changes

codecov[bot] avatar Jan 24 '24 10:01 codecov[bot]

@guedou When there are extra bytes after the IP packet (e.g., Ethernet trailer displayed as a Padding layer), it becomes part of q.payload which is appended to fragmentable. This causes fragmentable to include wrong data and to grow in size beyond the intended fragment data size. In this case warnings will be issued for the next fragment, for having a wrong offset field (since fragmentable became too large).

Example that triggers the issue:

pkt = Ether() / IPv6() / ICMPv6EchoRequest(data='b'*100)
frags = fragment6( pkt, 100 )
defragment6( frag / Padding('a'*8) for frag in frags )

My pull request correctly builds the fragment data that is appended to fragmentable.

shapaz avatar Jan 24 '24 12:01 shapaz

Thanks for your explanation. Could you add a unit test based on your example at https://github.com/secdev/scapy/blob/master/test/scapy/layers/inet6.uts#L1817 ?

guedou avatar Jan 26 '24 09:01 guedou