Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

WIP ICMP Extension Header bugfixes #4451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ventaquil wants to merge 1 commit into secdev:master
base: master
Choose a base branch
Loading
from ventaquil:bugfix/icmp-extensions

Conversation

@ventaquil
Copy link

@ventaquil ventaquil commented Jul 4, 2024

Continuing my work with ICMP Extension Header started with this issue GH-4281, I noticed that there are more bugs or missing mechanisms (like GH-4353).

I don't have enough time to do all things at once, also it would be nice to discuss some details during implementation to fulfill your expectations and met coding style.

I started with IPv4 variant, in incoming future it would be necessary to implement similar changes for IPv6.

At this moment I fixed:

  • Setting length attribute when extension header is present and field is not set.

The length attribute MUST be specified when the ICMP Extension Structure is appended to the above mentioned ICMP messages.

  • Accept payload longer than 128 octets.

When the ICMP Extension Structure is appended to an ICMP message and that ICMP message contains an "original datagram" field, the "original datagram" field MUST contain at least 128 octets.

To be done:

  • Set IPv6 length field.
  • Write tests with more scenarios and longer payloads.

RFC for reference - RFC 4884: Extended ICMP to Support Multi-Part Messages.

Copy link

codecov bot commented Jul 4, 2024

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 48.94%. Comparing base (a1afb9a) to head (b16c199).

❗ There is a different number of reports uploaded between BASE (a1afb9a) and HEAD (b16c199). Click for more details.

HEAD has 9 uploads less than BASE
Flag BASE (a1afb9a) HEAD (b16c199)
10 1
Additional details and impacted files
@@ Coverage Diff @@
## master #4451 +/- ##
===========================================
- Coverage 81.46% 48.94% -32.53% 
===========================================
 Files 353 346 -7 
 Lines 84477 78591 -5886 
===========================================
- Hits 68822 38467 -30355 
- Misses 15655 40124 +24469 
Files Coverage Δ
scapy/layers/inet.py 23.86% <0.00%> (-47.92%) ⬇️

... and 262 files with indirect coverage changes

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /