mirror of
https://github.com/github/codeql.git
synced 2026-01-29 22:32:58 +01:00
Merge pull request #445 from smowton/smowton/feature/git-as-shell
Add 'git' as a possible command-interpreter, unless arguments are sanitized using "--"
This commit is contained in:
Chris Smowton
@@ -14,7 +14,9 @@ import go
|
||||
import semmle.go.security.CommandInjection
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from CommandInjection::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where cfg.hasFlowPath(source, sink)
|
||||
from
|
||||
CommandInjection::Configuration cfg, CommandInjection::DoubleDashSanitizingConfiguration cfg2,
|
||||
DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where cfg.hasFlowPath(source, sink) or cfg2.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
|
||||
"a user-provided value"
|
||||
|
||||
@@ -21,6 +21,9 @@ class SystemCommandExecution extends DataFlow::Node {
|
||||
|
||||
/** Gets the argument that specifies the command to be executed. */
|
||||
DataFlow::Node getCommandName() { result = self.getCommandName() }
|
||||
|
||||
/** Holds if this node is sanitized whenever it follows `--` in an argument list. */
|
||||
predicate doubleDashIsSanitizing() { self.doubleDashIsSanitizing() }
|
||||
}
|
||||
|
||||
/** Provides a class for modeling new system-command execution APIs. */
|
||||
@@ -35,6 +38,9 @@ module SystemCommandExecution {
|
||||
abstract class Range extends DataFlow::Node {
|
||||
/** Gets the argument that specifies the command to be executed. */
|
||||
abstract DataFlow::Node getCommandName();
|
||||
|
||||
/** Holds if this node is sanitized whenever it follows `--` in an argument list. */
|
||||
predicate doubleDashIsSanitizing() { none() }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,12 +10,17 @@ import go
|
||||
* such as a shell, `sudo`, or a programming-language interpreter.
|
||||
*/
|
||||
private class ShellOrSudoExecution extends SystemCommandExecution::Range, DataFlow::CallNode {
|
||||
ShellLike shellCommand;
|
||||
|
||||
ShellOrSudoExecution() {
|
||||
this instanceof SystemCommandExecution and
|
||||
this.getAnArgument().getAPredecessor*() instanceof ShellLike
|
||||
shellCommand = this.getAnArgument().getAPredecessor*() and
|
||||
not hasSafeSubcommand(shellCommand.getStringValue(), this.getAnArgument().getStringValue())
|
||||
}
|
||||
|
||||
override DataFlow::Node getCommandName() { result = getAnArgument() }
|
||||
|
||||
override predicate doubleDashIsSanitizing() { shellCommand.getStringValue().matches("%git") }
|
||||
}
|
||||
|
||||
private class SystemCommandExecutors extends SystemCommandExecution::Range, DataFlow::CallNode {
|
||||
@@ -151,7 +156,39 @@ private string getASudoCommand() {
|
||||
result = "awk" or
|
||||
result = "gawk" or
|
||||
result = "mawk" or
|
||||
result = "nawk"
|
||||
result = "nawk" or
|
||||
result = "git"
|
||||
}
|
||||
|
||||
/**
|
||||
* Excuse git commands other than those that interact with remotes, as only those currently
|
||||
* take arbitrary commands to run on the remote host as arguments.
|
||||
*/
|
||||
bindingset[command, subcommand]
|
||||
private predicate hasSafeSubcommand(string command, string subcommand) {
|
||||
command.matches("%git") and
|
||||
// All git subcommands except for clone, fetch, ls-remote, pull and fetch-pack
|
||||
subcommand in [
|
||||
"add", "am", "archive", "bisect", "branch", "bundle", "checkout", "cherry-pick", "citool",
|
||||
"clean", "commit", "describe", "diff", "format-patch", "gc", "gitk", "grep", "gui", "init",
|
||||
"log", "merge", "mv", "notes", "push", "range-diff", "rebase", "reset", "restore", "revert",
|
||||
"rm", "shortlog", "show", "sparse-checkout", "stash", "status", "submodule", "switch", "tag",
|
||||
"worktree", "fast-export", "fast-import", "filter-branch", "mergetool", "pack-refs", "prune",
|
||||
"reflog", "remote", "repack", "replace", "annotate", "blame", "bugreport", "count-objects",
|
||||
"difftool", "fsck", "gitweb", "help", "instaweb", "merge-tree", "rerere", "show-branch",
|
||||
"verify-commit", "verify-tag", "whatchanged", "archimport", "cvsexportcommit", "cvsimport",
|
||||
"cvsserver", "imap-send", "p4", "quiltimport", "request-pull", "send-email", "apply",
|
||||
"checkout-index", "commit-graph", "commit-tree", "hash-object", "index-pack", "merge-file",
|
||||
"merge-index", "mktag", "mktree", "multi-pack-index", "pack-objects", "prune-packed",
|
||||
"read-tree", "symbolic-ref", "unpack-objects", "update-index", "update-ref", "write-tree",
|
||||
"cat-file", "cherry", "diff-files", "diff-index", "diff-tree", "for-each-ref",
|
||||
"get-tar-commit-id", "ls-files", "ls-tree", "merge-base", "name-rev", "pack-redundant",
|
||||
"rev-list", "rev-parse", "show-index", "show-ref", "unpack-file", "var", "verify-pack",
|
||||
"http-backend", "send-pack", "update-server-info", "check-attr", "check-ignore",
|
||||
"check-mailmap", "check-ref-format", "column", "credential", "credential-cache",
|
||||
"credential-store", "fmt-merge-msg", "interpret-trailers", "mailinfo", "mailsplit",
|
||||
"merge-one-file", "patch-id"
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -24,7 +24,9 @@ module CommandInjection {
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(Sink s | sink = s | not s.doubleDashIsSanitizing())
|
||||
}
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
super.isSanitizer(node) or
|
||||
@@ -35,4 +37,63 @@ module CommandInjection {
|
||||
guard instanceof SanitizerGuard
|
||||
}
|
||||
}
|
||||
|
||||
private class ArgumentArrayWithDoubleDash extends DataFlow::Node {
|
||||
int doubleDashIndex;
|
||||
|
||||
ArgumentArrayWithDoubleDash() {
|
||||
// Call whose argument list contains a "--"
|
||||
exists(DataFlow::CallNode c |
|
||||
this = c and
|
||||
(c = Builtin::append().getACall() or c = any(SystemCommandExecution sce)) and
|
||||
c.getArgument(doubleDashIndex).getStringValue() = "--"
|
||||
)
|
||||
or
|
||||
// array/slice literal containing a "--"
|
||||
exists(ArrayOrSliceLit litExpr |
|
||||
this = DataFlow::exprNode(litExpr) and
|
||||
litExpr.getElement(doubleDashIndex).getStringValue() = "--"
|
||||
)
|
||||
or
|
||||
// call consuming an existing an array with a "--"
|
||||
exists(ArgumentArrayWithDoubleDash alreadyHasDoubleDash, DataFlow::CallNode userCall |
|
||||
(
|
||||
alreadyHasDoubleDash.getType().getUnderlyingType() instanceof ArrayType or
|
||||
alreadyHasDoubleDash.getType() instanceof SliceType
|
||||
) and
|
||||
this = userCall and
|
||||
DataFlow::localFlow(alreadyHasDoubleDash, userCall.getArgument(doubleDashIndex))
|
||||
)
|
||||
}
|
||||
|
||||
DataFlow::Node getASanitizedElement() {
|
||||
exists(int sanitizedIndex |
|
||||
sanitizedIndex > doubleDashIndex and
|
||||
(
|
||||
result = this.(DataFlow::CallNode).getArgument(sanitizedIndex) or
|
||||
result = DataFlow::exprNode(this.asExpr().(ArrayOrSliceLit).getElement(sanitizedIndex))
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
class DoubleDashSanitizingConfiguration extends TaintTracking::Configuration {
|
||||
DoubleDashSanitizingConfiguration() { this = "CommandInjectionWithDoubleDashSanitizer" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(Sink s | sink = s | s.doubleDashIsSanitizing())
|
||||
}
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
super.isSanitizer(node) or
|
||||
node instanceof Sanitizer or
|
||||
node = any(ArgumentArrayWithDoubleDash array).getASanitizedElement()
|
||||
}
|
||||
|
||||
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
|
||||
guard instanceof SanitizerGuard
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,7 +19,10 @@ module CommandInjection {
|
||||
/**
|
||||
* A data flow sink for command-injection vulnerabilities.
|
||||
*/
|
||||
abstract class Sink extends DataFlow::Node { }
|
||||
abstract class Sink extends DataFlow::Node {
|
||||
/** Holds if this node is sanitized whenever it follows `--` in an argument list. */
|
||||
predicate doubleDashIsSanitizing() { none() }
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer for command-injection vulnerabilities.
|
||||
@@ -38,6 +41,10 @@ module CommandInjection {
|
||||
|
||||
/** A command name, considered as a taint sink for command injection. */
|
||||
class CommandNameAsSink extends Sink {
|
||||
CommandNameAsSink() { this = any(SystemCommandExecution exec).getCommandName() }
|
||||
SystemCommandExecution exec;
|
||||
|
||||
CommandNameAsSink() { this = exec.getCommandName() }
|
||||
|
||||
override predicate doubleDashIsSanitizing() { exec.doubleDashIsSanitizing() }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,73 @@
|
||||
edges
|
||||
| CommandInjection.go:9:13:9:19 | selection of URL : pointer type | CommandInjection.go:10:22:10:28 | cmdName |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:12:31:12:37 | tainted |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:13:31:13:37 | tainted |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:14:30:14:36 | tainted |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:15:35:15:41 | tainted |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:16:36:16:42 | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:14:23:14:33 | slice expression |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:40:23:40:30 | arrayLit |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:54:23:54:30 | arrayLit |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:70:23:70:30 | arrayLit |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:80:23:80:29 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:96:24:96:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:101:24:101:34 | slice expression |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:106:24:106:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:112:24:112:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:118:24:118:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:124:24:124:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:130:24:130:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:137:24:137:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:144:24:144:31 | arrayLit |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:148:30:148:36 | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:152:24:152:30 | tainted |
|
||||
nodes
|
||||
| CommandInjection.go:9:13:9:19 | selection of URL : pointer type | semmle.label | selection of URL : pointer type |
|
||||
| CommandInjection.go:10:22:10:28 | cmdName | semmle.label | cmdName |
|
||||
| GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | semmle.label | selection of URL : pointer type |
|
||||
| GitSubcommands.go:12:31:12:37 | tainted | semmle.label | tainted |
|
||||
| GitSubcommands.go:13:31:13:37 | tainted | semmle.label | tainted |
|
||||
| GitSubcommands.go:14:30:14:36 | tainted | semmle.label | tainted |
|
||||
| GitSubcommands.go:15:35:15:41 | tainted | semmle.label | tainted |
|
||||
| GitSubcommands.go:16:36:16:42 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | semmle.label | selection of URL : pointer type |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:40:23:40:30 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:54:23:54:30 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:70:23:70:30 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:80:23:80:29 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | semmle.label | selection of URL : pointer type |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice expression | semmle.label | slice expression |
|
||||
| SanitizingDoubleDash.go:106:24:106:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:112:24:112:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:118:24:118:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:124:24:124:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:130:24:130:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:137:24:137:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:144:24:144:31 | arrayLit | semmle.label | arrayLit |
|
||||
| SanitizingDoubleDash.go:148:30:148:36 | tainted | semmle.label | tainted |
|
||||
| SanitizingDoubleDash.go:152:24:152:30 | tainted | semmle.label | tainted |
|
||||
#select
|
||||
| CommandInjection.go:10:22:10:28 | cmdName | CommandInjection.go:9:13:9:19 | selection of URL : pointer type | CommandInjection.go:10:22:10:28 | cmdName | This command depends on $@. | CommandInjection.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| GitSubcommands.go:12:31:12:37 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:12:31:12:37 | tainted | This command depends on $@. | GitSubcommands.go:10:13:10:19 | selection of URL | a user-provided value |
|
||||
| GitSubcommands.go:13:31:13:37 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:13:31:13:37 | tainted | This command depends on $@. | GitSubcommands.go:10:13:10:19 | selection of URL | a user-provided value |
|
||||
| GitSubcommands.go:14:30:14:36 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:14:30:14:36 | tainted | This command depends on $@. | GitSubcommands.go:10:13:10:19 | selection of URL | a user-provided value |
|
||||
| GitSubcommands.go:15:35:15:41 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:15:35:15:41 | tainted | This command depends on $@. | GitSubcommands.go:10:13:10:19 | selection of URL | a user-provided value |
|
||||
| GitSubcommands.go:16:36:16:42 | tainted | GitSubcommands.go:10:13:10:19 | selection of URL : pointer type | GitSubcommands.go:16:36:16:42 | tainted | This command depends on $@. | GitSubcommands.go:10:13:10:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:14:23:14:33 | slice expression | SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:14:23:14:33 | slice expression | This command depends on $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:40:23:40:30 | arrayLit | SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:40:23:40:30 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:54:23:54:30 | arrayLit | SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:54:23:54:30 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:70:23:70:30 | arrayLit | SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:70:23:70:30 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:80:23:80:29 | tainted | SanitizingDoubleDash.go:9:13:9:19 | selection of URL : pointer type | SanitizingDoubleDash.go:80:23:80:29 | tainted | This command depends on $@. | SanitizingDoubleDash.go:9:13:9:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:96:24:96:34 | slice expression | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:96:24:96:34 | slice expression | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:101:24:101:34 | slice expression | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:101:24:101:34 | slice expression | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:106:24:106:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:106:24:106:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:112:24:112:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:112:24:112:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:118:24:118:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:118:24:118:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:124:24:124:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:124:24:124:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:130:24:130:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:130:24:130:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:137:24:137:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:137:24:137:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:144:24:144:31 | arrayLit | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:144:24:144:31 | arrayLit | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:148:30:148:36 | tainted | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:148:30:148:36 | tainted | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
| SanitizingDoubleDash.go:152:24:152:30 | tainted | SanitizingDoubleDash.go:92:13:92:19 | selection of URL : pointer type | SanitizingDoubleDash.go:152:24:152:30 | tainted | This command depends on $@. | SanitizingDoubleDash.go:92:13:92:19 | selection of URL | a user-provided value |
|
||||
|
||||
28
ql/test/query-tests/Security/CWE-078/GitSubcommands.go
Normal file
28
ql/test/query-tests/Security/CWE-078/GitSubcommands.go
Normal file
@@ -0,0 +1,28 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"os/exec"
|
||||
)
|
||||
|
||||
// BAD: using git subcommands that are vulnerable to arbitrary remote command execution
|
||||
func gitSubcommandsBad(req *http.Request) {
|
||||
tainted := req.URL.Query()["cmd"][0]
|
||||
|
||||
exec.Command("git", "clone", tainted)
|
||||
exec.Command("git", "fetch", tainted)
|
||||
exec.Command("git", "pull", tainted)
|
||||
exec.Command("git", "ls-remote", tainted)
|
||||
exec.Command("git", "fetch-pack", tainted)
|
||||
}
|
||||
|
||||
// GOOD: using a sampling of git subcommands that are not vulnerable to arbitrary remote command execution
|
||||
func gitSubcommandsGood(req *http.Request) {
|
||||
tainted := req.URL.Query()["cmd"][0]
|
||||
|
||||
exec.Command("git", "checkout", tainted)
|
||||
exec.Command("git", "branch", tainted)
|
||||
exec.Command("git", "diff", tainted)
|
||||
exec.Command("git", "merge", tainted)
|
||||
exec.Command("git", "add", tainted)
|
||||
}
|
||||
154
ql/test/query-tests/Security/CWE-078/SanitizingDoubleDash.go
Normal file
154
ql/test/query-tests/Security/CWE-078/SanitizingDoubleDash.go
Normal file
@@ -0,0 +1,154 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"os/exec"
|
||||
)
|
||||
|
||||
func testDoubleDashSanitizes(req *http.Request) {
|
||||
tainted := req.URL.Query()["cmd"][0]
|
||||
|
||||
// BAD: no sanitizing "--" preceding tainted data
|
||||
{
|
||||
arrayLit := [1]string{tainted}
|
||||
exec.Command("git", arrayLit[:]...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data
|
||||
{
|
||||
arrayLit := [2]string{"--", tainted}
|
||||
exec.Command("git", arrayLit[:]...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, as a slice
|
||||
{
|
||||
arrayLit := []string{"--", tainted}
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, added during an append
|
||||
{
|
||||
arrayLit := []string{}
|
||||
arrayLit = append(arrayLit, "--", tainted)
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// BAD: sanitizing "--" comes after tainted data, added during an append
|
||||
{
|
||||
arrayLit := []string{}
|
||||
arrayLit = append(arrayLit, tainted, "--")
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, built in two steps
|
||||
{
|
||||
arrayLit := []string{"--"}
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// BAD: sanitizing "--" comes after tainted data, built in two steps
|
||||
{
|
||||
arrayLit := []string{tainted}
|
||||
arrayLit = append(arrayLit, "--")
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, built in three steps
|
||||
{
|
||||
arrayLit := []string{"--"}
|
||||
arrayLit = append(arrayLit, "something else")
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// BAD: sanitizing "--" preceding tainted data, built in three steps
|
||||
{
|
||||
arrayLit := []string{"something else"}
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
arrayLit = append(arrayLit, "--")
|
||||
exec.Command("git", arrayLit...)
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, used directly in a Command
|
||||
{
|
||||
exec.Command("git", "--", tainted)
|
||||
}
|
||||
|
||||
// BAD: sanitizing "--" comes after tainted data, used directly in a Command
|
||||
{
|
||||
exec.Command("git", tainted, "--")
|
||||
}
|
||||
|
||||
// GOOD: sanitizing "--" preceding tainted data, used directly in a Command, after several other arguments
|
||||
{
|
||||
exec.Command("git", "--arg1", "--arg2", "--", tainted)
|
||||
}
|
||||
}
|
||||
|
||||
// This test mirrors testDoubleDashSanitizes above, but uses sudo instead of git, where "--" is not sanitizing.
|
||||
// All cases are therefore BAD.
|
||||
func testDoubleDashIrrelevant(req *http.Request) {
|
||||
tainted := req.URL.Query()["cmd"][0]
|
||||
|
||||
{
|
||||
arrayLit := [1]string{tainted}
|
||||
exec.Command("sudo", arrayLit[:]...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := [2]string{"--", tainted}
|
||||
exec.Command("sudo", arrayLit[:]...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{"--", tainted}
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{}
|
||||
arrayLit = append(arrayLit, "--", tainted)
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{}
|
||||
arrayLit = append(arrayLit, tainted, "--")
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{"--"}
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{tainted}
|
||||
arrayLit = append(arrayLit, "--")
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{"--"}
|
||||
arrayLit = append(arrayLit, "something else")
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
arrayLit := []string{"something else"}
|
||||
arrayLit = append(arrayLit, tainted)
|
||||
arrayLit = append(arrayLit, "--")
|
||||
exec.Command("sudo", arrayLit...)
|
||||
}
|
||||
|
||||
{
|
||||
exec.Command("sudo", "--", tainted)
|
||||
}
|
||||
|
||||
{
|
||||
exec.Command("sudo", tainted, "--")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user