DARWIN/MacOS patch to fix word alignment of netmask in sockaddr.sa_data and filter ARP entries#4895
Open
jhogg wants to merge 2 commits intosecdev:masterfrom
Open
DARWIN/MacOS patch to fix word alignment of netmask in sockaddr.sa_data and filter ARP entries#4895jhogg wants to merge 2 commits intosecdev:masterfrom
jhogg wants to merge 2 commits intosecdev:masterfrom
Conversation
(1) word alingmnent/endian-ness of netmask in routes (sockaddr) (2) remove host entries because the route read includes the entire ARP table.
gpotter2
reviewed
Jan 5, 2026
scapy/arch/bpf/pfroute.py
Outdated
| "sa_data", | ||
| "", | ||
| length_from=lambda pkt: pkt.sa_len - 2 if pkt.sa_len >= 2 else 0, | ||
| # NOTE: Darwin right-justifies netmask on 4 byte word alignment |
Member
There was a problem hiding this comment.
Could you use a PadField instead? I will check, but most likely this padding isn't DARWIN-specific.
What's the padding. 8?
scapy/arch/bpf/pfroute.py
Outdated
| if DARWIN and flags.RTF_WASCLONED and msg.rtm_parentflags.RTF_PRCLONING: | ||
| # OSX needs filtering | ||
| continue | ||
| if DARWIN and flags.RTF_HOST and not flags.RTF_BROADCAST: |
Member
There was a problem hiding this comment.
Could you do a
if DARWIN:
# OSX needs filtering
if ....:
continue
# <...>
if ....:
continue
for clarity? Thanks.
Member
|
Thanks for the PR ! |
…netmask) are all sockaddr_in format for BSD systems, and not just sockaddr, which adds a ushort port address in before the sa_data buffer. The challenge is that the sa_family is not INETi for the netmask, otherwise it would just work. Updated per review notes: - Changed structure to add 2 ushort buffer before sa_data unconditionally. - Changed math to always align to 4 byte boundary. Observations on DARWIN show the sa_len is not always correct, so modulus math was used. - Simpified the unpack and swapped all BSD's to use BE format, since all low level network is expected as BE. - Simplified the DARWIN route filtering per suggestion. This may have an impact on other systems than the original PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I added unit tests or explained why they are not relevant
These changes deal with low-level parsing of an OS response and unit tests were not found.
I executed the regression tests (using
tox)I was unable to execute tox -e flake8. AttributeError("module 'ast' has no attribute 'Str'")
This happened with requirements both <6.0.0 and current releases on 3.10. Beyond scope of patch to identify issue, but there are only 11 lines of code involved that are formatted accordingly.
(1) Fixes incorrect parsing of routing tables on MacOS. Issues with both word alignment and endian-ness.
(2) MacOS includes the entire ARP table within the route response. This patch filters the entire route table except a host entry for the default route, which MacOS identifies as 'CONNECTED' and is not a straight forward filter.
Enables routing using route tables correctly on MacOS, which could impact software expecting them to only use default route.
Fixes #4894