Skip to content

Commit 6294c3b

Browse files
committed
Remove Shellwords sanitizer in ql
Note that some sanitizers had no effect because flow through those functions wasn't modeled.
1 parent 4aee99f commit 6294c3b

File tree

2 files changed

+9
-12
lines changed

2 files changed

+9
-12
lines changed

ruby/ql/lib/codeql/ruby/security/CommandInjectionCustomizations.qll

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,6 @@ module CommandInjection {
4242
SystemCommandExecutionSink() { exists(SystemCommandExecution c | c.isShellInterpreted(this)) }
4343
}
4444

45-
/**
46-
* A call to `Shellwords.escape` or `Shellwords.shellescape` sanitizes its input.
47-
*/
48-
class ShellwordsEscapeAsSanitizer extends Sanitizer {
49-
ShellwordsEscapeAsSanitizer() {
50-
this = API::getTopLevelMember("Shellwords").getAMethodCall(["escape", "shellescape"])
51-
or
52-
// The method is also added as `String#shellescape`.
53-
this.(DataFlow::CallNode).getMethodName() = "shellescape"
54-
}
55-
}
56-
5745
private class ExternalCommandInjectionSink extends Sink {
5846
ExternalCommandInjectionSink() { ModelOutput::sinkNode(this, "command-injection") }
5947
}

ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
| CommandInjection.rb:83:14:83:34 | "echo #{...}" | CommandInjection.rb:82:23:82:33 | blah_number | CommandInjection.rb:83:14:83:34 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:82:23:82:33 | blah_number | user-provided value |
1414
| CommandInjection.rb:92:14:92:39 | "echo #{...}" | CommandInjection.rb:92:22:92:37 | ...[...] | CommandInjection.rb:92:14:92:39 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:92:22:92:37 | ...[...] | user-provided value |
1515
| CommandInjection.rb:105:16:105:28 | "cat #{...}" | CommandInjection.rb:104:16:104:21 | call to params | CommandInjection.rb:105:16:105:28 | "cat #{...}" | This command depends on a $@. | CommandInjection.rb:104:16:104:21 | call to params | user-provided value |
16+
| CommandInjection.rb:107:16:107:40 | "cat #{...}" | CommandInjection.rb:104:16:104:21 | call to params | CommandInjection.rb:107:16:107:40 | "cat #{...}" | This command depends on a $@. | CommandInjection.rb:104:16:104:21 | call to params | user-provided value |
1617
| CommandInjection.rb:112:33:112:44 | ...[...] | CommandInjection.rb:112:33:112:38 | call to params | CommandInjection.rb:112:33:112:44 | ...[...] | This command depends on a $@. | CommandInjection.rb:112:33:112:38 | call to params | user-provided value |
1718
| CommandInjection.rb:114:41:114:56 | "#{...}" | CommandInjection.rb:114:44:114:49 | call to params | CommandInjection.rb:114:41:114:56 | "#{...}" | This command depends on a $@. | CommandInjection.rb:114:44:114:49 | call to params | user-provided value |
1819
edges
@@ -36,8 +37,11 @@ edges
3637
| CommandInjection.rb:82:23:82:33 | blah_number | CommandInjection.rb:83:14:83:34 | "echo #{...}" | provenance | AdditionalTaintStep |
3738
| CommandInjection.rb:92:22:92:37 | ...[...] | CommandInjection.rb:92:14:92:39 | "echo #{...}" | provenance | AdditionalTaintStep |
3839
| CommandInjection.rb:104:9:104:12 | file | CommandInjection.rb:105:16:105:28 | "cat #{...}" | provenance | AdditionalTaintStep |
40+
| CommandInjection.rb:104:9:104:12 | file | CommandInjection.rb:107:23:107:26 | file | provenance | |
3941
| CommandInjection.rb:104:16:104:21 | call to params | CommandInjection.rb:104:16:104:28 | ...[...] | provenance | |
4042
| CommandInjection.rb:104:16:104:28 | ...[...] | CommandInjection.rb:104:9:104:12 | file | provenance | |
43+
| CommandInjection.rb:107:23:107:26 | file | CommandInjection.rb:107:23:107:38 | call to shellescape | provenance | |
44+
| CommandInjection.rb:107:23:107:38 | call to shellescape | CommandInjection.rb:107:16:107:40 | "cat #{...}" | provenance | AdditionalTaintStep |
4145
| CommandInjection.rb:112:33:112:38 | call to params | CommandInjection.rb:112:33:112:44 | ...[...] | provenance | Sink:MaD:1 |
4246
| CommandInjection.rb:114:44:114:49 | call to params | CommandInjection.rb:114:44:114:54 | ...[...] | provenance | |
4347
| CommandInjection.rb:114:44:114:54 | ...[...] | CommandInjection.rb:114:41:114:56 | "#{...}" | provenance | AdditionalTaintStep Sink:MaD:2 |
@@ -74,9 +78,14 @@ nodes
7478
| CommandInjection.rb:104:16:104:21 | call to params | semmle.label | call to params |
7579
| CommandInjection.rb:104:16:104:28 | ...[...] | semmle.label | ...[...] |
7680
| CommandInjection.rb:105:16:105:28 | "cat #{...}" | semmle.label | "cat #{...}" |
81+
| CommandInjection.rb:107:16:107:40 | "cat #{...}" | semmle.label | "cat #{...}" |
82+
| CommandInjection.rb:107:23:107:26 | file | semmle.label | file |
83+
| CommandInjection.rb:107:23:107:38 | call to shellescape | semmle.label | call to shellescape |
7784
| CommandInjection.rb:112:33:112:38 | call to params | semmle.label | call to params |
7885
| CommandInjection.rb:112:33:112:44 | ...[...] | semmle.label | ...[...] |
7986
| CommandInjection.rb:114:41:114:56 | "#{...}" | semmle.label | "#{...}" |
8087
| CommandInjection.rb:114:44:114:49 | call to params | semmle.label | call to params |
8188
| CommandInjection.rb:114:44:114:54 | ...[...] | semmle.label | ...[...] |
8289
subpaths
90+
testFailures
91+
| CommandInjection.rb:107:16:107:40 | "cat #{...}" | Unexpected result: Alert |

0 commit comments

Comments
 (0)