diff --git a/go/ql/lib/change-notes/2022-11-17-rsync-argument-injection.md b/go/ql/lib/change-notes/2022-11-17-rsync-argument-injection.md new file mode 100644 index 00000000000..fb39aebe09c --- /dev/null +++ b/go/ql/lib/change-notes/2022-11-17-rsync-argument-injection.md @@ -0,0 +1,4 @@ +--- + category: minorAnalysis +--- + * `rsync` has been added to the list of commands which may evaluate its parameters as a shell command. diff --git a/go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll b/go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll index c2d9929fa54..1e4b7637581 100644 --- a/go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll +++ b/go/ql/lib/semmle/go/frameworks/SystemCommandExecutors.qll @@ -20,7 +20,9 @@ private class ShellOrSudoExecution extends SystemCommandExecution::Range, DataFl override DataFlow::Node getCommandName() { result = this.getAnArgument() } - override predicate doubleDashIsSanitizing() { shellCommand.getStringValue().matches("%git") } + override predicate doubleDashIsSanitizing() { + shellCommand.getStringValue().matches("%" + ["git", "rsync"]) + } } private class SystemCommandExecutors extends SystemCommandExecution::Range, DataFlow::CallNode { @@ -126,7 +128,7 @@ private string getASudoCommand() { "fakeroot", "fakeroot-sysv", "su", "fakeroot-tcp", "fstab-decode", "jrunscript", "nohup", "parallel", "find", "pkexec", "sg", "sem", "runcon", "sudoedit", "runuser", "stdbuf", "system", "timeout", "xargs", "time", "awk", "gawk", "mawk", "nawk", "doas", "git", "access", - "vsys", "userv", "sus", "super" + "vsys", "userv", "sus", "super", "rsync" ] } diff --git a/go/ql/test/query-tests/Security/CWE-078/ArgumentInjection.go b/go/ql/test/query-tests/Security/CWE-078/ArgumentInjection.go new file mode 100644 index 00000000000..d38d4662542 --- /dev/null +++ b/go/ql/test/query-tests/Security/CWE-078/ArgumentInjection.go @@ -0,0 +1,12 @@ +package main + +import ( + "net/http" + "os/exec" +) + +func handler2(req *http.Request) { + path := req.URL.Query()["path"][0] + cmd := exec.Command("rsync", path, "/tmp") + cmd.Run() +} diff --git a/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected b/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected index 02c56107106..3c93a4657ca 100644 --- a/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected +++ b/go/ql/test/query-tests/Security/CWE-078/CommandInjection.expected @@ -1,4 +1,5 @@ edges +| ArgumentInjection.go:9:10:9:16 | selection of URL : pointer type | ArgumentInjection.go:10:31:10:34 | path | | 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 | @@ -25,6 +26,8 @@ edges | SanitizingDoubleDash.go:105:15:105:37 | slice literal [array] : string | SanitizingDoubleDash.go:106:24:106:31 | arrayLit | | SanitizingDoubleDash.go:105:30:105:36 | tainted : string | SanitizingDoubleDash.go:105:15:105:37 | slice literal [array] : string | nodes +| ArgumentInjection.go:9:10:9:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| ArgumentInjection.go:10:31:10:34 | path | semmle.label | path | | 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 | @@ -55,6 +58,7 @@ nodes | SanitizingDoubleDash.go:152:24:152:30 | tainted | semmle.label | tainted | subpaths #select +| ArgumentInjection.go:10:31:10:34 | path | ArgumentInjection.go:9:10:9:16 | selection of URL : pointer type | ArgumentInjection.go:10:31:10:34 | path | This command depends on a $@. | ArgumentInjection.go:9:10:9:16 | selection of URL | user-provided value | | 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 a $@. | CommandInjection.go:9:13:9:19 | selection of URL | 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 a $@. | GitSubcommands.go:10:13:10:19 | selection of URL | 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 a $@. | GitSubcommands.go:10:13:10:19 | selection of URL | user-provided value |