Skip to content

Commit a276cc3

Browse files
atorralbaatorralba
committed
Convert all command injection sinks to MaD format
1 parent f5070bb commit a276cc3

File tree

13 files changed

+81
-127
lines changed

13 files changed

+81
-127
lines changed

‎java/ql/lib/change-notes/2023-04-19-deprecated-execcallable.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: deprecated
3+
---
4+
* The `ExecCallable` class in `ExternalProcess.qll` has been deprecated.

‎java/ql/lib/ext/experimental/com.jcraft.jsch.model.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: experimentalSinkModel
5+
data:
6+
- ["com.jcraft.jsch", "ChannelExec", True, "setCommand", "", "", "Argument[0]", "command-injection", "manual", "jsch-os-injection"]

‎java/ql/lib/ext/java.lang.model.yml‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,20 @@ extensions:
88
- ["java.lang", "ClassLoader", True, "getSystemResource", "(String)", "", "Argument[0]", "read-file", "ai-manual"]
99
- ["java.lang", "ClassLoader", True, "getSystemResourceAsStream", "(String)", "", "Argument[0]", "read-file", "ai-manual"]
1010
- ["java.lang", "Module", True, "getResourceAsStream", "(String)", "", "Argument[0]", "read-file", "ai-manual"]
11-
# These are modeled in plain CodeQL. TODO: migrate them.
12-
# - ["java.lang", "ProcessBuilder", False, "command", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
13-
# - ["java.lang", "ProcessBuilder", False, "directory", "(File)", "", "Argument[0]", "command-injection", "ai-manual"]
14-
# - ["java.lang", "ProcessBuilder", False, "ProcessBuilder", "(List)", "", "Argument[0]", "command-injection", "ai-manual"]
15-
# - ["java.lang", "ProcessBuilder", False, "ProcessBuilder", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
16-
# - ["java.lang", "Runtime", True, "exec", "(String,String[])", "", "Argument[0]", "command-injection", "ai-manual"]
17-
# - ["java.lang", "Runtime", True, "exec", "(String[],String[])", "", "Argument[0]", "command-injection", "ai-manual"]
18-
# - ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[0]", "command-injection", "ai-manual"]
19-
# - ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[2]", "command-injection", "ai-manual"]
20-
# - ["java.lang", "Runtime", True, "exec", "(String)", "", "Argument[0]", "command-injection", "ai-manual"]
21-
# - ["java.lang", "Runtime", True, "exec", "(String[],String[],File)", "", "Argument[0]", "command-injection", "ai-manual"]
22-
# - ["java.lang", "Runtime", True, "exec", "(String[],String[],File)", "", "Argument[2]", "command-injection", "ai-manual"]
23-
# - ["java.lang", "Runtime", True, "exec", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
11+
- ["java.lang", "ProcessBuilder", False, "command", "(List)", "", "Argument[0]", "command-injection", "ai-manual"]
12+
- ["java.lang", "ProcessBuilder", False, "command", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
13+
- ["java.lang", "ProcessBuilder", False, "command", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
14+
- ["java.lang", "ProcessBuilder", False, "directory", "(File)", "", "Argument[0]", "command-injection", "ai-manual"]
15+
- ["java.lang", "ProcessBuilder", False, "ProcessBuilder", "(List)", "", "Argument[0]", "command-injection", "ai-manual"]
16+
- ["java.lang", "ProcessBuilder", False, "ProcessBuilder", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
17+
- ["java.lang", "Runtime", True, "exec", "(String)", "", "Argument[0]", "command-injection", "ai-manual"]
18+
- ["java.lang", "Runtime", True, "exec", "(String[])", "", "Argument[0]", "command-injection", "ai-manual"]
19+
- ["java.lang", "Runtime", True, "exec", "(String[],String[])", "", "Argument[0]", "command-injection", "ai-manual"]
20+
- ["java.lang", "Runtime", True, "exec", "(String[],String[],File)", "", "Argument[0]", "command-injection", "ai-manual"]
21+
- ["java.lang", "Runtime", True, "exec", "(String[],String[],File)", "", "Argument[2]", "command-injection", "ai-manual"]
22+
- ["java.lang", "Runtime", True, "exec", "(String,String[])", "", "Argument[0]", "command-injection", "ai-manual"]
23+
- ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[0]", "command-injection", "ai-manual"]
24+
- ["java.lang", "Runtime", True, "exec", "(String,String[],File)", "", "Argument[2]", "command-injection", "ai-manual"]
2425
- ["java.lang", "String", False, "matches", "(String)", "", "Argument[0]", "regex-use[f-1]", "manual"]
2526
- ["java.lang", "String", False, "replaceAll", "(String,String)", "", "Argument[0]", "regex-use[-1]", "manual"]
2627
- ["java.lang", "String", False, "replaceFirst", "(String,String)", "", "Argument[0]", "regex-use[-1]", "manual"]

‎java/ql/lib/ext/org.apache.commons.exec.model.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sinkModel
5+
data:
6+
- ["org.apache.commons.exec", "CommandLine", True, "parse", "(String)", "", "Argument[0]", "command-injection", "manual"]
7+
- ["org.apache.commons.exec", "CommandLine", True, "parse", "(String,Map)", "", "Argument[0]", "command-injection", "manual"]
8+
- ["org.apache.commons.exec", "CommandLine", True, "addArguments", "(String)", "", "Argument[0]", "command-injection", "manual"]
9+
- ["org.apache.commons.exec", "CommandLine", True, "addArguments", "(String,boolean)", "", "Argument[0]", "command-injection", "manual"]

‎java/ql/lib/semmle/code/java/JDK.qll‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
*/
44

55
import Member
6-
import semmle.code.java.security.ExternalProcess
76
private import semmle.code.java.dataflow.FlowSteps
87

98
// --- Standard types ---
@@ -198,39 +197,6 @@ class TypeFile extends Class {
198197
}
199198

200199
// --- Standard methods ---
201-
/**
202-
* Any constructor of class `java.lang.ProcessBuilder`.
203-
*/
204-
class ProcessBuilderConstructor extends Constructor, ExecCallable {
205-
ProcessBuilderConstructor() { this.getDeclaringType() instanceof TypeProcessBuilder }
206-
207-
override int getAnExecutedArgument() { result = 0 }
208-
}
209-
210-
/**
211-
* Any of the methods named `command` on class `java.lang.ProcessBuilder`.
212-
*/
213-
class MethodProcessBuilderCommand extends Method, ExecCallable {
214-
MethodProcessBuilderCommand() {
215-
this.hasName("command") and
216-
this.getDeclaringType() instanceof TypeProcessBuilder
217-
}
218-
219-
override int getAnExecutedArgument() { result = 0 }
220-
}
221-
222-
/**
223-
* Any method named `exec` on class `java.lang.Runtime`.
224-
*/
225-
class MethodRuntimeExec extends Method, ExecCallable {
226-
MethodRuntimeExec() {
227-
this.hasName("exec") and
228-
this.getDeclaringType() instanceof TypeRuntime
229-
}
230-
231-
override int getAnExecutedArgument() { result = 0 }
232-
}
233-
234200
/**
235201
* Any method named `getenv` on class `java.lang.System`.
236202
*/

‎java/ql/lib/semmle/code/java/frameworks/apache/Exec.qll‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎java/ql/lib/semmle/code/java/security/CommandLineQuery.qll‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
import java
1111
private import semmle.code.java.dataflow.FlowSources
1212
private import semmle.code.java.dataflow.ExternalFlow
13-
private import semmle.code.java.security.ExternalProcess
1413
private import semmle.code.java.security.CommandArguments
14+
private import semmle.code.java.security.ExternalProcess
1515

1616
/** A sink for command injection vulnerabilities. */
1717
abstract class CommandInjectionSink extends DataFlow::Node { }
@@ -33,9 +33,7 @@ class CommandInjectionAdditionalTaintStep extends Unit {
3333
}
3434

3535
private class DefaultCommandInjectionSink extends CommandInjectionSink {
36-
DefaultCommandInjectionSink() {
37-
this.asExpr() instanceof ArgumentToExec or sinkNode(this, "command-injection")
38-
}
36+
DefaultCommandInjectionSink() { sinkNode(this, "command-injection") }
3937
}
4038

4139
private class DefaultCommandInjectionSanitizer extends CommandInjectionSanitizer {
@@ -100,7 +98,7 @@ predicate execIsTainted(
10098
RemoteUserInputToArgumentToExecFlow::PathNode sink, Expr execArg
10199
) {
102100
RemoteUserInputToArgumentToExecFlow::flowPath(source, sink) and
103-
sink.getNode().asExpr() = execArg
101+
argumentToExec(execArg, sink.getNode())
104102
}
105103

106104
/**
@@ -112,7 +110,7 @@ predicate execIsTainted(
112110
*/
113111
deprecated predicate execTainted(DataFlow::PathNode source, DataFlow::PathNode sink, Expr execArg) {
114112
exists(RemoteUserInputToArgumentToExecFlowConfig conf |
115-
conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = execArg
113+
conf.hasFlowPath(source, sink) and argumentToExec(execArg, sink.getNode())
116114
)
117115
}
118116

‎java/ql/lib/semmle/code/java/security/ExternalProcess.qll‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
/** Definitions related to external processes. */
22

33
import semmle.code.java.Member
4-
5-
private module Instances {
6-
private import semmle.code.java.JDK
7-
private import semmle.code.java.frameworks.apache.Exec
8-
}
4+
private import semmle.code.java.dataflow.DataFlow
5+
private import semmle.code.java.security.CommandLineQuery
96

107
/**
11-
* A callable that executes a command.
8+
* DEPRECATED: A callable that executes a command.
129
*/
13-
abstract class ExecCallable extends Callable {
10+
abstract deprecated class ExecCallable extends Callable {
1411
/**
1512
* Gets the index of an argument that will be part of the command that is executed.
1613
*/
@@ -23,13 +20,19 @@ abstract class ExecCallable extends Callable {
2320
* to be executed.
2421
*/
2522
class ArgumentToExec extends Expr {
26-
ArgumentToExec() {
27-
exists(Call execCall, ExecCallable execCallable, int i |
28-
execCall.getArgument(pragma[only_bind_into](i)) = this and
29-
execCallable = execCall.getCallee() and
30-
i = execCallable.getAnExecutedArgument()
31-
)
32-
}
23+
ArgumentToExec() { argumentToExec(this, _) }
24+
}
25+
26+
/**
27+
* Holds if `e` is an expression used as an argument to a call that executes an external command.
28+
* For calls to varargs method calls, this only includes the first argument, which will be the command
29+
* to be executed.
30+
*/
31+
predicate argumentToExec(Expr e, CommandInjectionSink s) {
32+
s.asExpr() = e
33+
or
34+
e.(Argument).isNthVararg(0) and
35+
s.(DataFlow::ImplicitVarargsArray).getCall() = e.(Argument).getCall()
3336
}
3437

3538
/**

‎java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414

1515
import java
1616
import semmle.code.java.security.CommandLineQuery
17+
import semmle.code.java.security.ExternalProcess
1718
import LocalUserInputToArgumentToExecFlow::PathGraph
1819

1920
from
2021
LocalUserInputToArgumentToExecFlow::PathNode source,
21-
LocalUserInputToArgumentToExecFlow::PathNode sink
22-
where LocalUserInputToArgumentToExecFlow::flowPath(source, sink)
23-
select sink.getNode().asExpr(), source, sink, "This command line depends on a $@.",
24-
source.getNode(), "user-provided value"
22+
LocalUserInputToArgumentToExecFlow::PathNode sink, Expr e
23+
where
24+
LocalUserInputToArgumentToExecFlow::flowPath(source, sink) and
25+
argumentToExec(e, sink.getNode())
26+
select e, source, sink, "This command line depends on a $@.", source.getNode(),
27+
"user-provided value"

‎java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import java
1616
import semmle.code.java.security.CommandLineQuery
17+
import semmle.code.java.security.ExternalProcess
1718

1819
/**
1920
* Strings that are known to be sane by some simple local analysis. Such strings

0 commit comments

Comments
 (0)