diff --git a/change-notes/2020-11-11-stored-command.md b/change-notes/2020-11-11-stored-command.md new file mode 100644 index 00000000000..fed234b2d96 --- /dev/null +++ b/change-notes/2020-11-11-stored-command.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* A new query "Command built from stored data" (`go/stored-command`) has been added. The query detects command executions that contain data from a database or a similar possibly user-controllable source. diff --git a/ql/src/Security/CWE-078/StoredCommand.go b/ql/src/Security/CWE-078/StoredCommand.go new file mode 100644 index 00000000000..5b7c16d0c59 --- /dev/null +++ b/ql/src/Security/CWE-078/StoredCommand.go @@ -0,0 +1,16 @@ +package main + +import ( + "database/sql" + "os/exec" +) + +var db *sql.DB + +func run(query string) { + rows, _ := db.Query(query) + var cmdName string + rows.Scan(&cmdName) + cmd := exec.Command(cmdName) + cmd.Run() +} diff --git a/ql/src/Security/CWE-078/StoredCommand.qhelp b/ql/src/Security/CWE-078/StoredCommand.qhelp new file mode 100644 index 00000000000..6f08c8a9d68 --- /dev/null +++ b/ql/src/Security/CWE-078/StoredCommand.qhelp @@ -0,0 +1,45 @@ + + + + +

+If a system command invocation is built from stored data without sufficient sanitization, and that +data is stored from a user input, a malicious user may be able to run commands to exfiltrate data or +compromise the system. +

+
+ + +

+If possible, use hard-coded string literals to specify the command to run. Instead of interpreting +stored input directly as command names, examine the input and then choose among hard-coded string +literals. +

+

+If this is not possible, then add sanitization code to verify that the user input is safe before +using it. +

+
+ + +

+In the following example, the function run runs a command directly from the result of a +query: +

+ +

+The function extracts the name of a system command from the database query, and then runs it without +any further checks, which can cause a command-injection vulnerability. A possible solution is to +ensure that commands are checked against a whitelist: +

+ +
+ + +
  • +OWASP: Command Injection. +
  • +
    +
    diff --git a/ql/src/Security/CWE-078/StoredCommand.ql b/ql/src/Security/CWE-078/StoredCommand.ql new file mode 100644 index 00000000000..16430d8f501 --- /dev/null +++ b/ql/src/Security/CWE-078/StoredCommand.ql @@ -0,0 +1,20 @@ +/** + * @name Command built from stored data + * @description Building a system command from stored data that is user-controlled + * can lead to execution of malicious code by the user. + * @kind path-problem + * @problem.severity error + * @precision low + * @id go/stored-command + * @tags security + * external/cwe/cwe-078 + */ + +import go +import semmle.go.security.StoredCommand +import DataFlow::PathGraph + +from StoredCommand::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(), + "a stored value" diff --git a/ql/src/Security/CWE-078/StoredCommandGood.go b/ql/src/Security/CWE-078/StoredCommandGood.go new file mode 100644 index 00000000000..89aff705fb9 --- /dev/null +++ b/ql/src/Security/CWE-078/StoredCommandGood.go @@ -0,0 +1,18 @@ +package main + +import ( + "database/sql" + "os/exec" +) + +var db *sql.DB + +func run(query string) { + rows, _ := db.Query(query) + var cmdName string + rows.Scan(&cmdName) + if cmdName == "mybinary1" || cmdName == "mybinary2" { + cmd := exec.Command(cmdName) + } + cmd.Run() +} diff --git a/ql/src/semmle/go/security/StoredCommand.qll b/ql/src/semmle/go/security/StoredCommand.qll new file mode 100644 index 00000000000..114f772b614 --- /dev/null +++ b/ql/src/semmle/go/security/StoredCommand.qll @@ -0,0 +1,42 @@ +/** + * Provides a taint tracking configuration for reasoning about command + * injection vulnerabilities. + * + * Note, for performance reasons: only import this file if + * `StoredCommand::Configuration` is needed, otherwise + * `StoredCommandCustomizations` should be imported instead. + */ + +import go +import StoredXssCustomizations +import CommandInjectionCustomizations + +/** + * Provides a taint tracking configuration for reasoning about command + * injection vulnerabilities. + */ +module StoredCommand { + /** + * A taint-tracking configuration for reasoning about command-injection vulnerabilities. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "StoredCommand" } + + override predicate isSource(DataFlow::Node source) { + source instanceof StoredXss::Source and + // exclude file names, since those are not generally an issue + not source instanceof StoredXss::FileNameSource + } + + override predicate isSink(DataFlow::Node sink) { sink instanceof CommandInjection::Sink } + + override predicate isSanitizer(DataFlow::Node node) { + super.isSanitizer(node) or + node instanceof CommandInjection::Sanitizer + } + + override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { + guard instanceof CommandInjection::SanitizerGuard + } + } +} diff --git a/ql/test/query-tests/Security/CWE-078/StoredCommand.expected b/ql/test/query-tests/Security/CWE-078/StoredCommand.expected new file mode 100644 index 00000000000..e1aff8909d0 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-078/StoredCommand.expected @@ -0,0 +1,7 @@ +edges +| StoredCommand.go:11:2:11:27 | ... := ...[0] : pointer type | StoredCommand.go:14:22:14:28 | cmdName | +nodes +| StoredCommand.go:11:2:11:27 | ... := ...[0] : pointer type | semmle.label | ... := ...[0] : pointer type | +| StoredCommand.go:14:22:14:28 | cmdName | semmle.label | cmdName | +#select +| StoredCommand.go:14:22:14:28 | cmdName | StoredCommand.go:11:2:11:27 | ... := ...[0] : pointer type | StoredCommand.go:14:22:14:28 | cmdName | This command depends on $@. | StoredCommand.go:11:2:11:27 | ... := ...[0] | a stored value | diff --git a/ql/test/query-tests/Security/CWE-078/StoredCommand.go b/ql/test/query-tests/Security/CWE-078/StoredCommand.go new file mode 100644 index 00000000000..5b7c16d0c59 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-078/StoredCommand.go @@ -0,0 +1,16 @@ +package main + +import ( + "database/sql" + "os/exec" +) + +var db *sql.DB + +func run(query string) { + rows, _ := db.Query(query) + var cmdName string + rows.Scan(&cmdName) + cmd := exec.Command(cmdName) + cmd.Run() +} diff --git a/ql/test/query-tests/Security/CWE-078/StoredCommand.qlref b/ql/test/query-tests/Security/CWE-078/StoredCommand.qlref new file mode 100644 index 00000000000..9d5b9126095 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-078/StoredCommand.qlref @@ -0,0 +1 @@ +Security/CWE-078/StoredCommand.ql