Skip to content

Commit 5ef37c4

Browse files
owen-mcowen-mc
committed
Converting command-injection sinks to use MaD
1 parent 8536e7e commit 5ef37c4

File tree

7 files changed

+68
-78
lines changed

7 files changed

+68
-78
lines changed

‎go/ql/lib/ext/github.com.codeskyblue.go-sh.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/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["github.com/codeskyblue/go-sh", "", False, "Command", "", "", "Argument[0]", "command-injection", "manual"]
7+
- ["github.com/codeskyblue/go-sh", "Session", False, "Call", "", "", "Argument[0]", "command-injection", "manual"]
8+
- ["github.com/codeskyblue/go-sh", "Session", False, "Command", "", "", "Argument[0]", "command-injection", "manual"]
9+
- ["github.com/codeskyblue/go-sh", "Session", False, "Exec", "", "", "Argument[0]", "command-injection", "manual"]

‎go/ql/lib/ext/golang.org.x.crypto.ssh.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/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["golang.org/x/crypto/ssh", "Session", False, "CombinedOutput", "", "", "Argument[0]", "command-injection", "manual"]
7+
- ["golang.org/x/crypto/ssh", "Session", False, "Output", "", "", "Argument[0]", "command-injection", "manual"]
8+
- ["golang.org/x/crypto/ssh", "Session", False, "Run", "", "", "Argument[0]", "command-injection", "manual"]
9+
- ["golang.org/x/crypto/ssh", "Session", False, "Start", "", "", "Argument[0]", "command-injection", "manual"]

‎go/ql/lib/ext/os.exec.model.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["os/exec", "", False, "Command", "", "", "Argument[0]", "command-injection", "manual"]
7+
- ["os/exec", "", False, "CommandContext", "", "", "Argument[1]", "command-injection", "manual"]

‎go/ql/lib/ext/os.model.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ extensions:
33
pack: codeql/go-all
44
extensible: sinkModel
55
data:
6+
# path-injection
67
- ["os", "", False, "Chdir", "", "", "Argument[0]", "path-injection", "manual"]
78
- ["os", "", False, "Chmod", "", "", "Argument[0]", "path-injection", "manual"]
89
- ["os", "", False, "Chown", "", "", "Argument[0]", "path-injection", "manual"]
@@ -29,6 +30,8 @@ extensions:
2930
- ["os", "", False, "MkdirTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
3031
- ["os", "", False, "CreateTemp", "", "", "Argument[0..1]", "path-injection", "manual"]
3132
- ["os", "", False, "WriteFile", "", "", "Argument[0]", "path-injection", "manual"]
33+
# command-injection
34+
- ["os", "", False, "StartProcess", "", "", "Argument[0]", "command-injection", "manual"]
3235
- addsTo:
3336
pack: codeql/go-all
3437
extensible: summaryModel

‎go/ql/lib/ext/syscall.model.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
extensions:
2+
- addsTo:
3+
pack: codeql/go-all
4+
extensible: sinkModel
5+
data:
6+
- ["syscall", "", False, "Exec", "", "", "Argument[0]", "command-injection", "manual"]
7+
- ["syscall", "", False, "ForkExec", "", "", "Argument[0]", "command-injection", "manual"]
8+
- ["syscall", "", False, "StartProcess", "", "", "Argument[0]", "command-injection", "manual"]
9+
- ["syscall", "", False, "CreateProcess", "", "", "Argument[0]", "command-injection", "manual"]
10+
- ["syscall", "", False, "CreateProcessAsUser", "", "", "Argument[1]", "command-injection", "manual"]
211
- addsTo:
312
pack: codeql/go-all
413
extensible: summaryModel

‎go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll

Lines changed: 26 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,25 @@
55

66
import go
77

8+
private class DefaultSystemCommandExecution extends SystemCommandExecution::Range,
9+
DataFlow::CallNode
10+
{
11+
DataFlow::ArgumentNode commandName;
12+
13+
DefaultSystemCommandExecution() {
14+
sinkNode(commandName, "command-injection") and
15+
this = commandName.getCall()
16+
}
17+
18+
override DataFlow::Node getCommandName() {
19+
not commandName instanceof DataFlow::ImplicitVarargsSlice and
20+
result = commandName
21+
or
22+
commandName instanceof DataFlow::ImplicitVarargsSlice and
23+
result = this.getAnImplicitVarargsArgument()
24+
}
25+
}
26+
827
/**
928
* An indirect system-command execution via an argument argument passed to a command interpreter
1029
* such as a shell, `sudo`, or a programming-language interpreter.
@@ -26,85 +45,19 @@ private class ShellOrSudoExecution extends SystemCommandExecution::Range, DataFl
2645
}
2746
}
2847

29-
private class SystemCommandExecutors extends SystemCommandExecution::Range, DataFlow::CallNode {
30-
int cmdArg;
31-
32-
SystemCommandExecutors() {
33-
exists(string pkg, string name | this.getTarget().hasQualifiedName(pkg, name) |
34-
pkg = "os" and name = "StartProcess" and cmdArg = 0
35-
or
36-
// assume that if a `Cmd` is instantiated it will be run
37-
pkg = "os/exec" and name = "Command" and cmdArg = 0
38-
or
39-
pkg = "os/exec" and name = "CommandContext" and cmdArg = 1
40-
or
41-
// NOTE: syscall.ForkExec exists only on unix.
42-
// NOTE: syscall.CreateProcess and syscall.CreateProcessAsUser exist only on windows.
43-
pkg = "syscall" and
44-
name = ["Exec", "ForkExec", "StartProcess", "CreateProcess"] and
45-
cmdArg = 0
46-
or
47-
pkg = "syscall" and
48-
name = "CreateProcessAsUser" and
49-
cmdArg = 1
50-
)
51-
}
52-
53-
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(cmdArg) }
54-
}
55-
56-
/**
57-
* A call to the `Command` function, or `Call` or `Command` methods on a `Session` object
58-
* from the [go-sh](https://github.com/codeskyblue/go-sh) package, viewed as a
59-
* system-command execution.
60-
*/
61-
private class GoShCommandExecution extends SystemCommandExecution::Range, DataFlow::CallNode {
62-
GoShCommandExecution() {
63-
exists(string packagePath | packagePath = package("github.com/codeskyblue/go-sh", "") |
64-
// Catch method calls on the `Session` object:
65-
exists(Method method |
66-
method.hasQualifiedName(packagePath, "Session", "Call")
67-
or
68-
method.hasQualifiedName(packagePath, "Session", "Command")
69-
or
70-
method.hasQualifiedName(packagePath, "Session", "Exec")
71-
|
72-
this = method.getACall()
73-
)
74-
or
75-
// Catch calls to the `Command` function:
76-
this.getTarget().hasQualifiedName(packagePath, "Command")
77-
)
78-
}
79-
80-
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(0) }
81-
}
82-
8348
/**
49+
* DEPRECATED
50+
*
8451
* Provides classes for working with the
8552
* [golang.org/x/crypto/ssh](https://pkg.go.dev/golang.org/x/crypto/ssh) package.
8653
*/
87-
module CryptoSsh {
88-
/** Gets the package path `golang.org/x/crypto/ssh`. */
89-
string packagePath() { result = package("golang.org/x/crypto", "ssh") }
90-
54+
deprecated module CryptoSsh {
9155
/**
92-
* A call to a method on a `Session` object from the [ssh](golang.org/x/crypto/ssh)
93-
* package, viewed as a system-command execution.
56+
* DEPRECATED: Use `package("golang.org/x/crypto", "ssh")` instead.
57+
*
58+
* Gets the package path `golang.org/x/crypto/ssh`.
9459
*/
95-
private class SshCommandExecution extends SystemCommandExecution::Range, DataFlow::CallNode {
96-
SshCommandExecution() {
97-
// Catch method calls on the `Session` object:
98-
exists(Method method, string methodName |
99-
methodName = ["CombinedOutput", "Output", "Run", "Start"]
100-
|
101-
method.hasQualifiedName(packagePath(), "Session", methodName) and
102-
this = method.getACall()
103-
)
104-
}
105-
106-
override DataFlow::Node getCommandName() { result = this.getSyntacticArgument(0) }
107-
}
60+
deprecated string packagePath() { result = package("golang.org/x/crypto", "ssh") }
10861
}
10962

11063
/**

‎go/ql/src/Security/CWE-322/InsecureHostKeyCallback.ql

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@
1212

1313
import go
1414

15+
private string cryptoSshPackage() { result = package("golang.org/x/crypto", "ssh") }
16+
1517
/** The `ssh.InsecureIgnoreHostKey` function, which allows connecting to any host regardless of its host key. */
1618
class InsecureIgnoreHostKey extends Function {
17-
InsecureIgnoreHostKey() {
18-
this.hasQualifiedName(CryptoSsh::packagePath(), "InsecureIgnoreHostKey")
19-
}
19+
InsecureIgnoreHostKey() { this.hasQualifiedName(cryptoSshPackage(), "InsecureIgnoreHostKey") }
2020
}
2121

2222
/** An SSH host-key checking function. */
2323
class HostKeyCallbackFunc extends DataFlow::Node {
2424
HostKeyCallbackFunc() {
25-
exists(NamedType nt | nt.hasQualifiedName(CryptoSsh::packagePath(), "HostKeyCallback") |
25+
exists(NamedType nt | nt.hasQualifiedName(cryptoSshPackage(), "HostKeyCallback") |
2626
this.getType().getUnderlyingType() = nt.getUnderlyingType()
2727
) and
2828
// Restrict possible sources to either function definitions or
@@ -62,7 +62,7 @@ module Config implements DataFlow::ConfigSig {
6262
*/
6363
additional predicate writeIsSink(DataFlow::Node sink, Write write) {
6464
exists(Field f |
65-
f.hasQualifiedName(CryptoSsh::packagePath(), "ClientConfig", "HostKeyCallback") and
65+
f.hasQualifiedName(cryptoSshPackage(), "ClientConfig", "HostKeyCallback") and
6666
write.writesField(_, f, sink)
6767
)
6868
}

0 commit comments

Comments
 (0)