fix(net): guard NULL ifa_addr in IPv4 iface scan + validate extra-interface bridge name (1.1.24)#227
Merged
Merged
Conversation
…erface bridge name (1.1.24) Two low-severity, compile-gated fixes from the second-pass audit: - lib/net.cpp getIfaceIp4Addresses dereferenced a->ifa_addr->sa_family without a NULL check while the IPv6 sibling guards it. getifaddrs(3) can return entries with NULL ifa_addr (interface with no address, e.g. a fresh epair), so the IPv4 path could crash (local DoS). The loop now mirrors the IPv6 one: skip NULL ifa_addr, non-AF_INET, name mismatch, and NULL ifa_netmask. - lib/run.cpp extra-interfaces loop passed ex.bridgeIface straight into RunNet::createBridgeEpair -> `ifconfig <bridge> addm <member>` with no validation, while the primary bridge path runs it through BridgePure::validateBridgeName. A leading-'-' value is taken as an ifconfig(8) option (argument injection). The loop now validates the name the same way. validateBridgeName (rejects leading dash / shell metachars / .. / over-length) is already covered by bridge_pure_test; the net.cpp change is runtime-only. Bumps to 1.1.24; CHANGELOG + trust-model (en/uk). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK
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.
Summary
The two low-severity findings from the second-pass audit that don't need on-hardware validation — both are compile-gated by the FreeBSD build.
1. Null-pointer dereference in
getIfaceIp4Addresses—lib/net.cppThe IPv4 interface loop dereferenced
a->ifa_addr->sa_familywithout a NULL check, while the IPv6 sibling (getIfaceIp6Addresses) already guardsifa_addr == nullptr.getifaddrs(3)can return entries with a NULLifa_addr(an interface with no address — e.g. a freshly created epair before configuration), so the IPv4 path could crash (local DoS). The loop now mirrors the IPv6 one: it skips NULLifa_addr, non-AF_INET, name mismatches, and NULLifa_netmask.2. Extra-interface bridge name not validated —
lib/run.cppThe primary bridge path runs
optionNet->bridgeIfacethroughBridgePure::validateBridgeName, but the extra-interfaces loop passedex.bridgeIfacestraight intoRunNet::createBridgeEpair→ifconfig <bridge> addm <member>. A leading--value is taken as anifconfig(8)option (argument injection; no shell involved). The loop now validates the name the same way as the primary path.Testing
BridgePure::validateBridgeName(rejects the leading dash, shell metacharacters,.., and over-length names) is already covered bybridge_pure_test— I confirmed locally it rejects-abcand acceptsbridge0. Thenet.cppchange is runtime-only (no pure unit surface) and is compile-gated by the FreeBSD lite build.Remaining after this
The three items that genuinely need FreeBSD on-hardware validation stay deferred in
TODO: the lease-fileflock-on-renamed-inode race (network_lease*.cpp), therun.cppexception-path teardown ordering, and thesocket_proxy/proxypath confinement. These are blocked on the FreeBSD CI/validation infra (self-hosted runner or Cirrus), not on any further analysis.Version
Bumps to 1.1.24;
CHANGELOG.md+docs/trust-model.{md,uk.md}updated.🤖 Generated with Claude Code
Generated by Claude Code