Skip to content

Java: detect command injection through delegating launchers - #22818

Draft
owen-mc with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-is-safe-command-argument
Draft

owen-mc with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-is-safe-command-argument

Conversation

Copilot AI commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

isSafeCommandArgument treated non-first arguments as safe whenever the executable was not a recognized shell. Delegating launchers could therefore execute attacker-controlled arguments without producing a command-line injection alert.

  • Detection

    • Distinguish delegating launchers from shells.
    • Recognize env, sudo, xargs, chroot, nice, nohup, setsid, stdbuf, and timeout.
    • Apply detection to array literals, mutable arrays, and command lists.
  • Coverage

    • Add an alert case for each recognized launcher.
    • Preserve safe handling for non-delegating commands such as cp.
Runtime.getRuntime().exec(new String[]{"env", userControlledInput});

Copilot AI changed the title [WIP] Fix isSafeCommandArgument to account for delegating launchers Java: detect command injection through delegating launchers Oct 14, 2026
Copilot AI requested a review from owen-mc October 14, 2026 10:42
*/
private predicate isDelegatingLauncher(Expr ex) {
exists(string cmd | cmd = ex.(StringLiteral).getValue() |
cmd.regexpMatch("(.*/)?(chroot|env|nice|nohup|setsid|stdbuf|sudo|timeout|xargs)$")
@github-actions github-actions Bot added the Java label Oct 14, 2026
i > 0 and
not isShell(ai.getInit(0))
not isShell(ai.getInit(0)) and
not isDelegatingLauncher(ai.getInit(0))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new check disables sanitization for every argument after a launcher. For example:

new ProcessBuilder(new String[]{"env", "/bin/cp", userInput, "/tmp/destination"});

This now produces java/command-line-injection, although userInput remains a discrete argument to the fixed /bin/cp executable. This contradicts the safe direct-cp behavior tested at Test.java:48 and introduces false positives into a high-precision query.

Make launcher handling position-aware: flag the operand selecting the delegated command, then preserve normal safe-argument handling once a fixed non-shell command is known. Add GOOD cases for array literals, lists, and mutable arrays.

*/
private predicate isDelegatingLauncher(Expr ex) {
exists(string cmd | cmd = ex.(StringLiteral).getValue() |
cmd.regexpMatch("(.*/)?(chroot|env|nice|nohup|setsid|stdbuf|sudo|timeout|xargs)$")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common launchers such as flock, su, runuser, doas, and time remain unrecognized. For example, taint in {"flock", "/tmp/lock", userInput} is still sanitized even though userInput selects the executed command. This matches the existing unresolved review thread.

@owen-mc

owen-mc commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@copilot Please address the three comments on this PR.

Copilot AI commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please address the three comments on this PR.

Addressed all three comments in edb67fe and 866d93b. Launcher handling now preserves safe arguments after fixed non-shell commands, recognizes the missing wrappers, and includes GOOD cases for literals, lists, and mutable arrays. Added safeguards and regressions for unresolved commands and mutated aliases. All four CWE-078 tests pass; final read-only review found no issues. The automated security scan was skipped because its database was too large.

Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Fixed
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Fixed
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Fixed
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

6 open findings
What changed in this PR

Updates the Java CWE-078 CodeQL modeling and tests to better distinguish delegating “launcher” executables (for example env, sudo, timeout) from shells, improving command-injection and relative-path detection accuracy.

Changes:

  • Extend isSafeCommandArgument to handle delegating launchers by analyzing fixed command prefixes across arrays/lists/mutable arrays.
  • Add new test cases covering launcher delegation scenarios and update expected query outputs accordingly.
  • Introduce new argument-sequence abstraction (CommandArguments) and supporting utilities for alias/escape tracking.
File Description
java/​ql/​test/​query-tests/​security/​CWE-078/​Test.java Adds new ProcessBuilder test cases for delegating launchers, argument mutation/aliasing, and fixed-vs-tainted scenarios.
java/​ql/​test/​query-tests/​security/​CWE-078/​ExecTainted.expected Updates expected taint-tracking results to include new test cases and shifted line numbers.
java/​ql/​test/​query-tests/​security/​CWE-078/​ExecRelative.expected Updates expected relative-path command results for newly added launcher cases and shifted line numbers.
java/​ql/​lib/​semmle/​code/​java/​security/​CommandArguments.qll Implements launcher-aware safe-argument logic, including prefix inspection and container escape tracking.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Outdated
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Outdated
Comment on lines +166 to +186
launcher = this.getFixedArgument(_).regexpCapture("(.*/)?([^/]+)", 2) and
options = [true, false] and
exists(Expr arg | arg = this.getArgument(index) |
(options = false or not this.getFixedArgument(index).matches("-%")) and
not (launcher = "env" and this.getFixedArgument(index).matches("%=%")) and
result = index
or
options = true and
this.getFixedArgument(index) = "--" and
result = this.commandOperand(launcher, index + 1, false)
or
options = true and
exists(int width |
launcherOption(launcher, this.getFixedArgument(index), width) and
(width = 1 or exists(this.getArgument(index + 1))) and
result = this.commandOperand(launcher, index + width, true)
)
or
launcher = "env" and
this.getFixedArgument(index).matches("%=%") and
result = this.commandOperand(launcher, index + 1, options)
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Outdated
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Outdated
Comment thread java/ql/test/query-tests/security/CWE-078/Test.java Outdated
@owen-mc

owen-mc commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

@copilot Address review comments.

Copilot AI commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Address review comments.

Addressed in 8f39336. Fixed the launcher operand binding and review findings, added .exe launcher coverage, and made the dynamic array index safe.

@owen-mc
owen-mc force-pushed the copilot/fix-is-safe-command-argument branch 3 times, most recently from 4edf3dd to f1d360c Compare October 16, 2026 13:58
Document supported simple forms and known limitations with inline expectations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@owen-mc
owen-mc force-pushed the copilot/fix-is-safe-command-argument branch from f1d360c to f627764 Compare October 16, 2026 16:13
Comment thread java/ql/lib/semmle/code/java/security/CommandArguments.qll Fixed
@owen-mc
owen-mc force-pushed the copilot/fix-is-safe-command-argument branch from f627764 to 78f0ba2 Compare October 16, 2026 18:19
Recognize fixed command positions without parsing launcher options. Only sanitize later arguments when the delegated command is one unchanged string literal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@owen-mc
owen-mc force-pushed the copilot/fix-is-safe-command-argument branch from 78f0ba2 to 8f59bbb Compare October 16, 2026 18:22

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants