Java: Should these sanitizer heuristics completely stop taint in five security queries?
- 主要語言
- CodeQL
- 星號
- 10.1k
- 分支
- 2.1k
- 平均合併
- 2 天 15 小時
- 30 天內合併 PR
- 141
描述
**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
```java
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:
```java
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:
```java
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
```java
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
```java
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
```java
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 `alert(1)`, 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:
```text
codeql database create db --language=java \
--source-root=. --command='javac *.java'
codeql database analyze db \
--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:
```java
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
貢獻指南
研究方向
首先使用 CodeQL CLI 重現 Java witness cases,然後閱讀連結位置的 LogInjection.qll、RequestForgery.qll、CommandArguments.qll 和 XSS.qll。完成這項工作需要確定建模方向,在適當情況下修正 sanitizer 或參數處理,並新增涵蓋所報告範例的 BAD 案例和安全控制。
由索引模型根據 Issue 內容生成。
評估
- 技術堆疊
- java
- 領域
- security
- Issue 類型
- 缺陷
- 難度
- 5/5
- 預估耗時
- 一週以上
- 活躍度
- 冷清
- 描述清晰度
- 基本清楚
- 新手友好度
- 28/100