pkg/ip, host-local: fix panic on a RangeStart at the top of a family - #1293
Open
thc1006 wants to merge 2 commits into
Open
pkg/ip, host-local: fix panic on a RangeStart at the top of a family#1293thc1006 wants to merge 2 commits into
thc1006 wants to merge 2 commits into
Conversation
{"ipam":{"type":"host-local","ranges":[[{"subnet":"255.255.255.252/30","rangeStart":"255.255.255.255","gateway":"255.255.255.255"}]]}}
LoadIPAMConfig accepts this config. RangeEnd is not set, so Canonicalize
defaults it to 255.255.255.254, one below the explicit RangeStart. The
first call to Get skips RangeStart because it equals Gateway, asks the
iterator for the next address, and NextIP(255.255.255.255) carries into
a 5th byte. intToIP then computes make([]byte, 4-5) and panics with
"makeslice: len out of range", so a single malformed CNI ADD config can
crash the plugin. The same shape reaches an all-ones IPv6 RangeStart.
Two gaps let this through:
1. intToIP assumes the incremented value fits in the address family's
byte width and never checks the overflow direction. NextIP's own doc
comment says invalid input returns nil, but intToIP didn't hold to
that once the carry needed more bytes than the family, so the nil
contract only worked for the too-few-bytes case fixed by containernetworking#782/containernetworking#783.
2. Range.Canonicalize validates RangeStart against Contains before
RangeEnd has been assigned a value (explicit or defaulted), so an
explicit RangeStart that only conflicts with the *defaulted* RangeEnd
was never checked against it.
Fix intToIP to return nil when the carry overflows the family width,
matching NextIP's contract. Fix Canonicalize to reject RangeStart >
RangeEnd once both are resolved, which also covers the general case of
an explicit RangeStart landing on a subnet's broadcast address with a
defaulted RangeEnd, not just the top of the whole address family.
Tests: NextIP cases for the last address of both families, a
Canonicalize case for RangeStart landing after a defaulted RangeEnd
(both a plain subnet and the top-of-family case), and a LoadIPAMConfig
regression test using the exact reported config.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
thc1006
force-pushed
the
pkg-ip-host-local-topofspace-panic
branch
from
August 17, 2026 10:20
264e981 to
d6d4b09
Compare
intToIP decided the result length from the minimal big.Int encoding, so a low IPv6 value whose leading bytes are zero (NextIP(::ff:ffff)) came back as a 4-byte IPv4 address. And because big.Int.Bytes returns the absolute value, PrevIP below the first address of a family returned the wrapped value instead of nil. Decide the family width first, reject sign-negative underflow and oversized overflow, and always return the family's fixed width. Add regressions for IPv6 family preservation and both family minima. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
This was referenced Aug 17, 2026
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
1 participant
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.
Fixes #1292.
intToIPconverts arithmetic results back to fixed-widthnet.IPvalues. It must preserve the selected address family and reject results outside that family.Incrementing the final IPv4 or IPv6 address produces a value one byte wider than the family, which makes the padding calculation use a negative length and panic. The existing conversion also returned a four-byte IPv4 value when a valid IPv6 result happened to have exactly four significant bytes (for example
NextIP(::ff:ffff)returned1.0.0.0rather than::100:0), and becausebig.Int.Bytesreturns the absolute value, a subtraction below the first address of a family came back as the wrapped value instead ofnil.The host-local reproducer in #1292 is IPv4-specific. An explicit IPv4
rangeStartat the subnet broadcast address can be greater than the defaultrangeEnd, which excludes broadcast.Range.CanonicalizecheckedRangeStartbefore assigning the default end, so the reversed range was accepted. At255.255.255.255this reachesNextIPand panics; in a lower subnet a broadcastrangeStartwalks outside the configured range before it reaches a family boundary. IPv6 defaultsrangeEndto the subnet's final address, so an all-onesrangeStartis alsorangeEndand the iterator exhausts the range before callingNextIP; the directNextIP(max IPv6)helper call has the same arithmetic overflow, which the new unit test covers.Changes:
pkg/ip: decide the family width first, reject sign-negative underflow and oversized overflow, and always return the selected family's fixed width.host-local: rejectRangeStart > RangeEndafter both values have been resolved.Tests cover both family maxima, IPv6 family-width preservation, both family minima for
PrevIP, the defaulted-end ordering check, and the exact configuration from #1292.