spring-projects / spring-projects/spring-security

IpInetAddressMatcher throws ArrayIndexOutOfBoundsException instead of returning false for mismatched IPv4/IPv6 families

Open Beginner friendly
#19,733 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.