java/ssrf: allowlist guard not recognized when expressed via Stream/lambda (anyMatch), only via plain equals()/for-loop

[richard-alun-williams]

Query

java/ssrf (Server-side request forgery), Java/Kotlin Security/CWE-918

Summary

The SSRF sanitizer-guard recognition for this query does not appear to model an allowlist check expressed via Arrays.stream(...).anyMatch(...) (or other Stream/lambda/method-reference based equality checks) as taint-clearing, even though a functionally identical check written as a plain for loop with .equals()/.equalsIgnoreCase() calls is recognized and clears the alert.

Example (not recognized — alert fires)

private static boolean isAllowedHost(String host, String... allowedHosts) {
    return Arrays.stream(allowedHosts).anyMatch(host::equalsIgnoreCase);
}

void fetch(String userUrl) {
    URI uri = new URI(userUrl);
    if (isAllowedHost(uri.getHost(), "example.com")) {
        uri.toURL().openConnection(); // still flagged as SSRF sink
    }
}

Example (recognized — alert clears)

private static boolean isAllowedHost(String host, String... allowedHosts) {
    for (String allowed : allowedHosts) {
        if (allowed.equalsIgnoreCase(host)) {
            return true;
        }
    }
    return false;
}

(Identical behavior/contract; only the loop construct differs.)

Why this matters

Lambda-based and Stream-based collection idioms (anyMatch, Set.of(...).contains(...), etc.) have been idiomatic, widely-used Java since Java 8 (2014). A sanitizer-guard recognizer that only matches a narrow inline if (LITERAL.equals(x))/plain-loop shape — and not equivalent Stream/lambda forms — produces false positives that push teams toward less readable, less idiomatic code purely to satisfy the analyzer, with no corresponding security benefit. This also seems inconsistent with sanitizer/guard modeling in other CodeQL queries that do recognize Collection.contains(...)-style checks.

Ask

Could the java/ssrf (and ideally the shared sanitizer-guard library used across similar taint-tracking queries) be extended to recognize equality-based allowlist checks expressed via common Stream/lambda idioms (Arrays.stream(...).anyMatch(x::equals), Collection.contains(x), Set.of(...).contains(x)) as taint-clearing guards, equivalent to their imperative-loop counterparts?

Happy to provide a minimal reproducible test case/repo if useful.

[richard-alun-williams]

Update: found the exact sanitizer definition, root cause is narrower/more specific than originally reported

Traced this further and found the precise sanitizer predicate in java/ql/lib/semmle/code/java/security/RequestForgery.qll (HostComparisonSanitizer / isHostComparisonSanitizer):

private predicate isHostComparisonSanitizer(Guard guard, Expr e, boolean branch) {
  guard =
    any(MethodCall equalsCall |
      equalsCall.getMethod().getName() = "equals" and
      branch = true and
      exists(MethodCall hostCall |
        hostCall = [equalsCall.getQualifier(), equalsCall.getArgument(0)] and
        hostCall.getMethod().hasQualifiedName("java.net", "URI", "getHost") and
        e = hostCall.getQualifier()
      )
    )
}

This is considerably narrower than my original report suggested. It requires both:

1. The method name must be exactly "equals". equalsIgnoreCase() - despite being semantically equivalent for this purpose (host comparison) and arguably the more correct choice, since DNS hostnames are case-insensitive - is not recognized at all.
2. uri.getHost() must be called directly inline as the qualifier or argument of that .equals() call. Reading the host into a local variable first (String host = uri.getHost(); ... x.equals(host)) breaks the match, even though this is an extremely common, arguably more readable idiom (especially once you're doing more than a single one-line comparison, e.g. looping over multiple allowed hosts).

So the original reproduction I gave (Arrays.stream(...).anyMatch(host::equalsIgnoreCase)) actually fails this pattern for two independent reasons, not one: it's both (a) a case-insensitive comparison and (b) working off a pre-extracted local variable, on top of (c) being expressed via Stream/method-reference rather than a literal AST-level .equals() call.

Concretely, none of these are recognized as sanitizing the taint on uri:

String host = uri.getHost();
if (allowedHost.equals(host)) { ... }                       // (b) fails - host is a local var, not inline getHost() call
if (allowedHost.equalsIgnoreCase(uri.getHost())) { ... }     // (a) fails - wrong method name
Arrays.stream(allowedHosts).anyMatch(uri.getHost()::equals); // (c) fails - method reference, not literal equals() call

Only this exact shape works:

if (allowedHost.equals(uri.getHost())) { ... }               // sink guarded here, in the true branch

Ask (revised)

Given how many semantically-identical, arguably-more-idiomatic variants fall outside this pattern, could the HostComparisonSanitizer be broadened to:

• also recognize equalsIgnoreCase() (hostnames are case-insensitive by spec - RFC 4343 - so this is actually the more correct comparison method, not a lesser one),
• track uri.getHost() through simple local-variable assignment (basic SSA/def-use, which CodeQL's data-flow libraries already support elsewhere), and
• recognize the same comparison performed via common Stream/Collection idioms (Arrays.stream(hosts).anyMatch(host::equalsIgnoreCase), Set.of(hosts).contains(host)), not just a single inline literal call.

Happy to share the specific before/after code from our fix if a concrete Java sample would help validate a broadened predicate.