Skip to content
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

IpAddressMatcher null pointer exception #15527

Closed
hananbs opened this issue Aug 6, 2024 · 2 comments
Closed

IpAddressMatcher null pointer exception #15527

hananbs opened this issue Aug 6, 2024 · 2 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@hananbs
Copy link

hananbs commented Aug 6, 2024

Previously we used IpAddressMatcher for matching ips.
After upgrade to Spring boot 3.3, my tests start failing on cases I provide null as 'address'. due to internal checks NPE is thrown when null address supplied.

https://github.com/spring-projects/spring-security/blob/main/web/src/main/java/org/springframework/security/web/util/matcher/IpAddressMatcher.java#L103

in previous version (SB3.1) when null was provided it was internally handled as localhost ip ("localhost/127.0.0.1" InnetAddress).
https://github.com/spring-projects/spring-security/blob/main/web/src/main/java/org/springframework/security/web/util/matcher/IpAddressMatcher.java#L109

To Reproduce
Spring framework: 6.1.10
Spring boot: 3.3.1

perform:
new IpAddressMatcher().matches(null)

Expected behavior
spring matcher should internally consider null as localhost

Thanks in advance.
If this intention to not have default assumption over null please let me know.
I did not found it in any release note/ migration guide

@hananbs hananbs added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Aug 6, 2024
@ankith2301
Copy link

this issue is resolved i tried reproducing this is resolved

@sjohnr sjohnr self-assigned this Nov 14, 2024
@sjohnr sjohnr added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 14, 2024
@sjohnr sjohnr closed this as completed in 52de894 Nov 15, 2024
@sjohnr sjohnr added this to the 6.2.8 milestone Nov 15, 2024
@sjohnr sjohnr moved this to In Progress in Spring Security Team Nov 15, 2024
@sjohnr sjohnr modified the milestones: 6.2.8, 6.4.0, 6.3.5 Nov 15, 2024
@sjohnr sjohnr moved this from In Progress to Done in Spring Security Team Nov 15, 2024
sjohnr added a commit that referenced this issue Nov 15, 2024
Closes gh-15527

(cherry picked from commit 52de894)
sjohnr added a commit that referenced this issue Nov 15, 2024
@sjohnr
Copy link
Member

sjohnr commented Nov 15, 2024

@hananbs thanks for reporting this. I have just pushed a fix to main to fix this bug. I was not clear on which branches exhibited the bug but eventually found that it was introduced in 6.3 so I backported the fix to 6.3.x as well, and it should be available in 6.3.5 on Monday (Nov 18, 2024).

Unfortunately, there are no tests asserting that null is a valid input to the matches() method and we don't want to rely on InetAddress.getByName(null) to define the behavior here. So IpAddressMatcher#matches((String) null) returns false including for "127.0.0.1". This is now codified in a test.

I should also mention that a similar behavior was exhibited by the constructor so I have also added an assertion to the constructor that requires a non-empty input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
Status: Done
Development

No branches or pull requests

3 participants