spring-projects / spring-projects/spring-security
IpInetAddressMatcher throws ArrayIndexOutOfBoundsException instead of returning false for mismatched IPv4/IPv6 families
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
Describe the bug
IpInetAddressMatcher.matches(InetAddress) (introduced in 7.1, backing IpAddressMatcher/InetAddressMatchers) can throw ArrayIndexOutOfBoundsException instead of returning false when the address being checked and the configured address are of different IP families (IPv4 vs. IPv6).
The class-level Javadoc on IpAddressMatcher states:
a matcher which is configured with an IPv4 address will never match a request which returns an IPv6 address, and vice-versa.
The current implementation doesn't enforce this at the byte-array level:
byte[] remAddr = toCheck.getAddress();
byte[] reqAddr = this.requiredAddress.getAddress();
int nMaskFullBytes = this.nMaskBits / 8;
byte finalByte = (byte) (0xFF00 >> (this.nMaskBits & 0x07));
for (int i = 0; i < nMaskFullBytes; i++) {
if (remAddr[i] != reqAddr[i]) {
return false;
}
}
There's no check that remAddr.length == reqAddr.length. If the configured CIDR mask requires more full bytes than the shorter of the two addresses has (e.g. an IPv6 /64 matcher checked against an IPv4 address), and the leading bytes happen to be equal up to the shorter array's length, the loop indexes past the end of the shorter array.
To Reproduce
IpAddressMatcher matcher = new IpAddressMatcher("2001:db8::/64");
// 32.1.13.184's bytes (0x20, 0x01, 0x0d, 0xb8) equal the first 4 bytes of 2001:0db8::,
// so the comparison loop doesn't short-circuit before running past the 4-byte array.
matcher.matches("32.1.13.184");
This throws:
java.lang.ArrayIndexOutOfBoundsException: Index 4 out of bounds for length 4
at org.springframework.security.util.matcher.IpInetAddressMatcher.matches(IpInetAddressMatcher.java:100)
instead of returning false.
Expected behavior
matches() should return false when the two addresses belong to different families, per the documented contract, instead of throwing.
Sample
I have a fix ready (add a length check on the two byte arrays before comparing them) plus regression tests, and will open a PR referencing this issue.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at IpInetAddressMatcher.matches(InetAddress), reported at IpInetAddressMatcher.java:100, and inspect how the configured and checked address byte arrays are compared. Add regression coverage for mismatched IPv4 and IPv6 families, including the reproducing addresses. Done means matches() returns false instead of throwing an ArrayIndexOutOfBoundsException.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100