Skip to content

Commit 739661e

Browse files
committed
Test that KernelMethodCall is specific enough
Calls to `UnknownModule.system`, where `UnknownModule` is a module that we know nothing about, should not be identified as instances of `KernelMethodCall`.
1 parent 64a8ced commit 739661e

File tree

4 files changed

+22
-17
lines changed

4 files changed

+22
-17
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ module CommandInjection {
4848
*/
4949
class ShellwordsEscapeAsSanitizer extends Sanitizer {
5050
ShellwordsEscapeAsSanitizer() {
51-
this = API::getTopLevelMember("Shellwords").getAMethodCall("escape") or
52-
this = API::getTopLevelMember("Shellwords").getAMethodCall("shellescape")
51+
this = API::getTopLevelMember("Shellwords").getAMethodCall(["escape", "shellescape"])
5352
}
5453
}
5554
}

ql/test/library-tests/frameworks/CommandExecution.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,5 @@ def run
8686
MockSystem.system("ls")
8787
end
8888
end
89+
90+
UnknownModule.system("ls")

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,30 @@ edges
44
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:10:14:10:16 | cmd |
55
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:11:17:11:22 | #{...} |
66
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:13:9:13:14 | #{...} |
7-
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:26:19:26:24 | #{...} |
8-
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:30:24:30:36 | "echo #{...}" |
9-
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:31:39:31:51 | "grep #{...}" |
10-
| CommandInjection.rb:43:15:43:20 | call to params : | CommandInjection.rb:47:24:47:36 | "echo #{...}" |
7+
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:29:19:29:24 | #{...} |
8+
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:33:24:33:36 | "echo #{...}" |
9+
| CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:34:39:34:51 | "grep #{...}" |
10+
| CommandInjection.rb:46:15:46:20 | call to params : | CommandInjection.rb:50:24:50:36 | "echo #{...}" |
1111
nodes
1212
| CommandInjection.rb:6:15:6:20 | call to params : | semmle.label | call to params : |
1313
| CommandInjection.rb:7:10:7:15 | #{...} | semmle.label | #{...} |
1414
| CommandInjection.rb:8:16:8:18 | cmd | semmle.label | cmd |
1515
| CommandInjection.rb:10:14:10:16 | cmd | semmle.label | cmd |
1616
| CommandInjection.rb:11:17:11:22 | #{...} | semmle.label | #{...} |
1717
| CommandInjection.rb:13:9:13:14 | #{...} | semmle.label | #{...} |
18-
| CommandInjection.rb:26:19:26:24 | #{...} | semmle.label | #{...} |
19-
| CommandInjection.rb:30:24:30:36 | "echo #{...}" | semmle.label | "echo #{...}" |
20-
| CommandInjection.rb:31:39:31:51 | "grep #{...}" | semmle.label | "grep #{...}" |
21-
| CommandInjection.rb:43:15:43:20 | call to params : | semmle.label | call to params : |
22-
| CommandInjection.rb:47:24:47:36 | "echo #{...}" | semmle.label | "echo #{...}" |
18+
| CommandInjection.rb:29:19:29:24 | #{...} | semmle.label | #{...} |
19+
| CommandInjection.rb:33:24:33:36 | "echo #{...}" | semmle.label | "echo #{...}" |
20+
| CommandInjection.rb:34:39:34:51 | "grep #{...}" | semmle.label | "grep #{...}" |
21+
| CommandInjection.rb:46:15:46:20 | call to params : | semmle.label | call to params : |
22+
| CommandInjection.rb:50:24:50:36 | "echo #{...}" | semmle.label | "echo #{...}" |
23+
subpaths
2324
#select
2425
| CommandInjection.rb:7:10:7:15 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:7:10:7:15 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
2526
| CommandInjection.rb:8:16:8:18 | cmd | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:8:16:8:18 | cmd | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
2627
| CommandInjection.rb:10:14:10:16 | cmd | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:10:14:10:16 | cmd | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
2728
| CommandInjection.rb:11:17:11:22 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:11:17:11:22 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
2829
| CommandInjection.rb:13:9:13:14 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:13:9:13:14 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
29-
| CommandInjection.rb:26:19:26:24 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:26:19:26:24 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
30-
| CommandInjection.rb:30:24:30:36 | "echo #{...}" | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:30:24:30:36 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
31-
| CommandInjection.rb:31:39:31:51 | "grep #{...}" | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:31:39:31:51 | "grep #{...}" | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
32-
| CommandInjection.rb:47:24:47:36 | "echo #{...}" | CommandInjection.rb:43:15:43:20 | call to params : | CommandInjection.rb:47:24:47:36 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:43:15:43:20 | call to params | a user-provided value |
30+
| CommandInjection.rb:29:19:29:24 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:29:19:29:24 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
31+
| CommandInjection.rb:33:24:33:36 | "echo #{...}" | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:33:24:33:36 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
32+
| CommandInjection.rb:34:39:34:51 | "grep #{...}" | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:34:39:34:51 | "grep #{...}" | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
33+
| CommandInjection.rb:50:24:50:36 | "echo #{...}" | CommandInjection.rb:46:15:46:20 | call to params : | CommandInjection.rb:50:24:50:36 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:46:15:46:20 | call to params | a user-provided value |

ql/test/query-tests/security/cwe-078/CommandInjection.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ def create
1313
#{cmd}
1414
EOF
1515

16-
safe_cmd = Shellwords.escape(cmd)
17-
`echo #{safe_cmd}`
16+
safe_cmd_1 = Shellwords.escape(cmd)
17+
`echo #{safe_cmd_1}`
18+
19+
safe_cmd_2 = Shellwords.shellescape(cmd)
20+
`echo #{safe_cmd_2}`
1821

1922
if cmd == "some constant"
2023
`echo #{cmd}`

0 commit comments

Comments
 (0)