Skip to content

Addressing comments#253

Open
KJ202 wants to merge 7 commits into
rwinch:google-ssrffrom
KJ202:google-ssrf
Open

Addressing comments#253
KJ202 wants to merge 7 commits into
rwinch:google-ssrffrom
KJ202:google-ssrf

Conversation

@KJ202

@KJ202 KJ202 commented Feb 17, 2026

Copy link
Copy Markdown

Description

This PR addresses code review feedback from spring-projects#18634 by refactoring and expanding the internal IP address matching and parsing logic.

A Test-Driven Development (TDD) approach was used to ensure all specific address spaces mentioned in the feedback (169.254.x.x, 172.16.x.x/12, etc.) are caught accurately while improving overall code simplicity and safety.

Changes Incorporated

  • Expanded Link-Local Coverage: Updated InternalInetAddressMatcher to naturally identify link-local addresses (such as 169.254.169.254) as internal to help prevent common SSRF bypass vectors.
  • Resolved Partial 172.x Subnet Checking: Replaced manual byte parsing of the 172.16 range with the native isSiteLocalAddress utility. This seamlessly expands coverage so the entire Class B /12 subnet (out to 172.31.255.255) is now properly evaluated as internal.
  • IPv4-Mapped IPv6 Coverage: Ensuring reliance on the established java.net standard library safeguards allows IPv4-Mapped IPv6 loopback and private subnets (e.g., ::ffff:192.168.1.1) to be properly evaluated as internal addresses rather than being used as a bypass element.
  • Preserved IPv6 Unique Local Address Checks:
    • Note to Reviewer: We maintained our manual byte-array block explicitly checking the fc00::/7 IPv6 Unique Local Address space.
    • This is required because the Inet6Address standard library continues to strictly adhere to the long-deprecated fec0::/10 standard (RFC 2373) and returns false natively for modern ULAs (RFC 4193). Preserving this block ensures these private IPv6 scopes are not accidentally treated as external public IPs.
  • Defensive IP Parsing: Updated InetAddressParser with defensive StringUtils.hasText checks to prevent the underlying charAt operations from throwing unexpected StringIndexOutOfBoundsExceptions when evaluating blank or empty string inputs.

Testing

  • Added parameter tests covering 169.254.x.x link-local spaces.
  • Added parameter tests covering ranges up to 172.31.255.255.
  • Added boundary testing isolating InetAddressParser exception handling.
  • All tests successfully pass.

rwinch and others added 7 commits February 2, 2026 16:19
@rwinch @vasporig
@KJ202 @rstoyanchev
Add InetAddressMatcher
d82601f 
Co-authored-by: Gábor Vaspöri <gabor.vaspori@gmail.com>
Co-authored-by: Kian Jamali <kianjamali123@gmail.com>
Co-authored-by: Rossen Stoyanchev <rstoyanchev@users.noreply.github.com>
@rwinch
Use consistent match terminology
7d614ac 
Previously the terminology blurred allow/deny into the matching APIs
which was confusing. The API now consistently uses matching logic.
@rwinch
Remove unnecessary since tag
59984df
@rwinch
IpInetAddressMatcher Should Use Authors from IpAddressMatcher
46120d2
@rwinch
Update InetAddressMatcher.matches(InetAddress) to allow null
d85df0c 
This is more consistent with the matches(String) method and it allows APIs
like ServerHttpRequest.getRemoteAddress() to be passed in without needing
to check if the value is null.
@rwinch
IpAddressServerWebExchangeMatcher uses InetAddressMatcher
6510838 
IpAddressServerWebExchangeMatcher was used for WebFlux applications which
used IpAddressMatcher. This was likely problematic because IpAddressMatcher
contains servlet APIs in it's method signatures. Even if those specific
methods aren't used by IpAddressServerWebExchangeMatcher it can cause
classpath issues for webflux applications.

This commit switches to using InetAddressMatcher which does not contain
any dependencies on the ServletAPI
@KJ202
Refactor IP address matching to address PR spring-projects#18634 feed…
2735507 
…back

- Use `isLinkLocalAddress()` to correctly identify link-local addresses (e.g., 169.254.x.x) as internal.
- Use `isSiteLocalAddress()` to correctly match the entire 172.16.0.0/12 subnet and rely on standard JVM checks for IPv4-mapped IPv6 addresses.
- Retain custom matching for Unique Local Addresses (fc00::/7) due to JVM limitations with RFC 4193.
- Add defensive blank string checking in `InetAddressParser.isIpAddress()`.
@rwinch rwinch force-pushed the google-ssrf branch 3 times, most recently from 3c8c183 to d693ad9 Compare February 19, 2026 17:29
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.

2 participants

@KJ202 @rwinch