Java: Should these sanitizer heuristics completely stop taint in five security queries?

[wildoranges]

Description of the issue

I am trying to understand whether the following Java sanitizer heuristics are
intended to act as complete barriers. Each one suppresses a concrete result
even though the security effect relevant to the query remains possible. The
examples below show the behavior and ask whether the models can be narrowed.

Query	Model	Baseline alerts	Reference alerts
java/log-injection	LineBreaksLogInjectionSanitizer	0	1
java/unvalidated-url-redirection	ContainsUrlSanitizer	0	1
java/unvalidated-url-redirection	RegexpCheckRequestForgerySanitizer	0	1
java/command-line-injection	isSafeCommandArgument	0	1
java/xss	HtmlEscapeXssSanitizer	0	1
java/ssrf	ContainsUrlSanitizer	0	1

All six results were reproduced with CodeQL CLI 2.25.6 and the CodeQL Java
queries at commit f6f45d1536312f53eed079868e344a5906bf3d72. The relevant
models were also checked and remain present on upstream main at commit
a8a77a60a68540662688e3e63e93021a8b8ed74a (2026-07-30); the analyzer counts
below are from the tested f6f45d1 snapshot.

1. java/log-injection: removing only one line-break character

logInjectionSanitizer considers a String.replace call sanitizing when its
target is either CR or LF and its replacement does not contain CR/LF:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/LogInjection.qll#L66-L92

import java.util.logging.Logger;
import javax.servlet.http.HttpServletRequest;

class Witness {
    static void handler(HttpServletRequest request) {
        String input = request.getParameter("q");
        String sanitized = input.replace("\r", "");
        Logger.getLogger("x").info("user: " + sanitized);
    }
}

An input containing attack\nforged retains the LF and can still forge a log
line. java/log-injection reports no alert. Removing only the replace call
produces one alert at the same logger sink.

Question: is removal of only one line-break character intended to be a complete
sanitizer? Would it be reasonable to require a composition that removes or
escapes both CR and LF before treating the value as fully sanitized?

2. ContainsUrlSanitizer: any method named contains

isContainsUrlSanitizer matches the first argument of any method named
contains; the implementation comment acknowledges that it also matches
unrelated methods:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/RequestForgery.qll#L95-L118

The following user-defined method performs only a bypassable prefix check:

import javax.servlet.http.*;

class RedirectWitness {
    static class UrlChecker {
        boolean contains(String url) {
            return url != null && url.startsWith("https://trusted.com");
        }
    }

    void handle(HttpServletRequest req, HttpServletResponse resp)
        throws Exception {
        String url = req.getParameter("next");
        if (new UrlChecker().contains(url)) {
            resp.sendRedirect(url);
        }
    }
}

https://trusted.com.evil.com passes this check but redirects to the external
host trusted.com.evil.com. The java/unvalidated-url-redirection query
reports zero alerts. Renaming the method to isAllowed, without changing its
implementation or call site semantics, produces one alert.

The same model also suppresses java/ssrf for a server-side request:

import java.net.URL;
import java.net.URLConnection;
import javax.servlet.http.*;

class SsrfWitness {
    static class HostChecker {
        boolean contains(String url) {
            return url != null && url.startsWith("https://trusted.com");
        }
    }

    void handle(HttpServletRequest req) throws Exception {
        String url = req.getParameter("target");
        if (new HostChecker().contains(url)) {
            URLConnection connection = new URL(url).openConnection();
            connection.connect();
        }
    }
}

Again, the baseline reports zero alerts and an otherwise identical method named
isAllowed produces one java/ssrf alert.

Question: is matching unknown user-defined methods by simple name intended?
Would it be reasonable to restrict this sanitizer to known collection types
and require the receiver to represent a trusted, fixed allowlist, while
retaining taint for unrelated methods named contains?

3. URL regexp checks treated as complete sanitizers

RegexpCheckRequestForgerySanitizer treats a regexp check as a complete
RequestForgerySanitizer:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/RequestForgery.qll#L151-L155

import javax.servlet.http.*;

class Witness {
    void handle(HttpServletRequest req, HttpServletResponse resp)
        throws Exception {
        String url = req.getParameter("next");
        if (url.matches("https://trusted\\.com.*")) {
            resp.sendRedirect(url);
        }
    }
}

The regexp accepts https://trusted.com.evil.com, whose host is external.
java/unvalidated-url-redirection nevertheless reports no alert. Removing only
the guard produces one alert.

Question: is a regexp check intended to be a complete sanitizer when it permits
the external-host bypass above? Would it be reasonable to retain URL taint
unless the regexp establishes an appropriate destination property?

4. java/command-line-injection: delegating launchers

isSafeCommandArgument treats a non-first argument as safe whenever the first
array element is not recognized as a shell:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/CommandArguments.qll#L11-L30

The shell recognition is a fixed name regexp and does not include delegating
launchers such as POSIX env:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/CommandArguments.qll#L33-L39

import java.io.*;
import java.net.Socket;

class Witness {
    static void handler(Socket socket) throws Exception {
        BufferedReader reader = new BufferedReader(
            new InputStreamReader(socket.getInputStream()));
        String input = reader.readLine();
        Runtime.getRuntime().exec(new String[]{"env", input});
    }
}

On POSIX systems, the env utility executes its first non-option operand as a
command. Therefore an input such as uname selects the effective executable;
it is not inert data. The java/command-line-injection query reports no alert
for the baseline. A direct-execution control using
Runtime.getRuntime().exec(input) produces one alert, as do controls using
{"sh", input} or {input, "x"}. The direct-execution control changes the
launcher topology and is corroborating analyzer evidence rather than a
semantics-preserving program transformation.

Question: should arguments interpreted as commands by delegating launchers be
excluded from isSafeCommandArgument? Would it be reasonable to recognize
common delegating executables such as POSIX env, or avoid treating every
argument to an unknown non-shell executable as unconditionally safe?

5. java/xss: sanitizer recognition based only on method name

HtmlEscapeXssSanitizer treats the return value of any method whose name
matches (?i)html_?escape.* as sanitized:

https://github.com/github/codeql/blob/f6f45d1536312f53eed079868e344a5906bf3d72/java/ql/lib/semmle/code/java/security/XSS.qll#L69-L76

import javax.servlet.http.*;

class Witness {
    static String htmlEscape(String value) {
        return value; // no-op
    }

    void handle(HttpServletRequest req, HttpServletResponse resp)
        throws Exception {
        String input = req.getParameter("q");
        resp.getWriter().write(htmlEscape(input));
    }
}

For input <script>alert(1)</script>, the method returns the payload unchanged,
but java/xss reports no alert. Renaming the method to sanitizeOutput while
leaving the implementation identical produces one alert.

Question: is method-name-only recognition intended to cover arbitrary
application methods? Would it be reasonable to model known framework escaping
APIs by canonical symbol identity while retaining taint for unknown
user-defined methods?

Reproduction procedure

For the command-line example, create the database with its Java source:

codeql database create db --language=java \
  --source-root=. --command='javac *.java'
codeql database analyze db <query.ql> \
  --format=sarif-latest --output=baseline.sarif

The log-injection, URL, XSS, and SSRF examples use minimal
javax.servlet.http interfaces with the correct fully qualified names during
extraction. The required declarations are:

package javax.servlet.http;

import java.io.PrintWriter;

public interface HttpServletRequest {
    String getParameter(String name);
}

public interface HttpServletResponse {
    void sendRedirect(String location) throws java.io.IOException;
    PrintWriter getWriter() throws java.io.IOException;
}

Place each public interface in its corresponding file under
javax/servlet/http/, compile it with the witness, and run the query listed in
the summary table. For each reference arm, make only the change described in
the relevant section and recreate the database. Each baseline has zero results
and each reference has one result.

Questions / possible model directions

• Could the log-injection sanitizer require complete CR/LF neutralization?
• Could ContainsUrlSanitizer be restricted to known collection/allowlist
  forms rather than every method named contains?
• Could regexp checks retain URL taint unless they establish a relevant
  destination property?
• Could delegating launchers be accounted for in command-argument
  classification?
• Could XSS sanitizer recognition use canonical models of known escaping APIs
  rather than method names alone?

If these models are narrowed, could the examples above be added as BAD cases
while retaining genuinely safe controls for numeric types, correct host
comparisons, and real HTML escaping?

Environment

• CodeQL CLI: 2.25.6
• CodeQL repository commit:
  f6f45d1536312f53eed079868e344a5906bf3d72
• Java/Javac: OpenJDK 21.0.11
• Platform: Linux/amd64

[oscarsj]

Thank you for this thorough and well-researched report. The witness programs, reproduction steps, and per-case analysis make this straightforward to triage.

I'm forwarding this to the Java query team and adding it to our backlog. I'll let you know if we have questions as we work through each case.