Skip to content

Commit 8f65d78

Browse files
committed
Add Shellwords.escape as CLI injection sanitizer
1 parent fe8fc06 commit 8f65d78

File tree

3 files changed

+30
-15
lines changed

3 files changed

+30
-15
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import codeql.ruby.DataFlow
88
private import codeql.ruby.dataflow.RemoteFlowSources
99
private import codeql.ruby.Concepts
1010
private import codeql.ruby.Frameworks
11+
private import codeql.ruby.ApiGraphs
1112

1213
module CommandInjection {
1314
/**
@@ -38,7 +39,17 @@ module CommandInjection {
3839
/**
3940
* A command argument to a function that initiates an operating system command.
4041
*/
41-
class SystemCommandExecutionSink extends Sink, DataFlow::Node {
42+
class SystemCommandExecutionSink extends Sink {
4243
SystemCommandExecutionSink() { this = any(SystemCommandExecution c).getAnArgument() }
4344
}
45+
46+
/**
47+
* A call to `Shellwords.escape` or `Shellwords.shellescape` sanitizes its input.
48+
*/
49+
class ShellwordsEscapeAsSanitizer extends Sanitizer {
50+
ShellwordsEscapeAsSanitizer() {
51+
this = API::getTopLevelMember("Shellwords").getAMethodCall("escape") or
52+
this = API::getTopLevelMember("Shellwords").getAMethodCall("shellescape")
53+
}
54+
}
4455
}
Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
edges
2-
| CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:4:10:4:15 | #{...} |
3-
| CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:5:16:5:18 | cmd |
4-
| CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:6:14:6:16 | cmd |
5-
| CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:7:12:7:17 | #{...} |
2+
| CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:6:10:6:15 | #{...} |
3+
| CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:7:16:7:18 | cmd |
4+
| CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:8:14:8:16 | cmd |
5+
| CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:9:17:9:22 | #{...} |
66
nodes
7-
| CommandInjection.rb:3:15:3:20 | call to params : | semmle.label | call to params : |
8-
| CommandInjection.rb:4:10:4:15 | #{...} | semmle.label | #{...} |
9-
| CommandInjection.rb:5:16:5:18 | cmd | semmle.label | cmd |
10-
| CommandInjection.rb:6:14:6:16 | cmd | semmle.label | cmd |
11-
| CommandInjection.rb:7:12:7:17 | #{...} | semmle.label | #{...} |
7+
| CommandInjection.rb:5:15:5:20 | call to params : | semmle.label | call to params : |
8+
| CommandInjection.rb:6:10:6:15 | #{...} | semmle.label | #{...} |
9+
| CommandInjection.rb:7:16:7:18 | cmd | semmle.label | cmd |
10+
| CommandInjection.rb:8:14:8:16 | cmd | semmle.label | cmd |
11+
| CommandInjection.rb:9:17:9:22 | #{...} | semmle.label | #{...} |
1212
#select
13-
| CommandInjection.rb:4:10:4:15 | #{...} | CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:4:10:4:15 | #{...} | This command depends on $@. | CommandInjection.rb:3:15:3:20 | call to params | a user-provided value |
14-
| CommandInjection.rb:5:16:5:18 | cmd | CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:5:16:5:18 | cmd | This command depends on $@. | CommandInjection.rb:3:15:3:20 | call to params | a user-provided value |
15-
| CommandInjection.rb:6:14:6:16 | cmd | CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:6:14:6:16 | cmd | This command depends on $@. | CommandInjection.rb:3:15:3:20 | call to params | a user-provided value |
16-
| CommandInjection.rb:7:12:7:17 | #{...} | CommandInjection.rb:3:15:3:20 | call to params : | CommandInjection.rb:7:12:7:17 | #{...} | This command depends on $@. | CommandInjection.rb:3:15:3:20 | call to params | a user-provided value |
13+
| CommandInjection.rb:6:10:6:15 | #{...} | CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:6:10:6:15 | #{...} | This command depends on $@. | CommandInjection.rb:5:15:5:20 | call to params | a user-provided value |
14+
| CommandInjection.rb:7:16:7:18 | cmd | CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:7:16:7:18 | cmd | This command depends on $@. | CommandInjection.rb:5:15:5:20 | call to params | a user-provided value |
15+
| CommandInjection.rb:8:14:8:16 | cmd | CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:8:14:8:16 | cmd | This command depends on $@. | CommandInjection.rb:5:15:5:20 | call to params | a user-provided value |
16+
| CommandInjection.rb:9:17:9:22 | #{...} | CommandInjection.rb:5:15:5:20 | call to params : | CommandInjection.rb:9:17:9:22 | #{...} | This command depends on $@. | CommandInjection.rb:5:15:5:20 | call to params | a user-provided value |

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1+
require "shellwords"
2+
13
class UsersController < ActionController::Base
24
def create
35
cmd = params[:cmd]
46
`#{cmd}`
57
system(cmd)
68
exec(cmd)
7-
%x(#{cmd})
9+
%x(echo #{cmd})
10+
safe_cmd = Shellwords.escape(cmd)
11+
`echo #{safe_cmd}`
812
end
913

1014
def show

0 commit comments

Comments
 (0)