Skip to content

Commit 7b63493

Browse files
hmachmac
committed
Ruby: Fix identification IO.open args
1 parent 79c6dc1 commit 7b63493

File tree

3 files changed

+38
-30
lines changed

3 files changed

+38
-30
lines changed

‎ruby/ql/lib/codeql/ruby/frameworks/core/IO.qll

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ module IO {
122122
override predicate isShellInterpreted(DataFlow::Node arg) { this.argument(arg, true) }
123123

124124
/**
125-
* A helper predicate that holds if `arg` is an argument to this call. `shell` is true if the argument is passed to a subshell.
125+
* Holds if `arg` is an argument to this call. `shell` is true if the argument is passed to a subshell.
126126
*/
127127
private predicate argument(DataFlow::Node arg, boolean shell) {
128128
exists(ExprCfgNode n | n = arg.asExpr() |
@@ -136,13 +136,15 @@ module IO {
136136
// This increases the sensitivity of the CommandInjection query at the risk of some FPs.
137137
if n.getConstantValue().getString() = "-" then shell = false else shell = true
138138
or
139-
// IO.popen({var: "a"}, [{var: "b"}, "cmd", "arg1", "arg2", {some: :opt}])
139+
// IO.popen([{var: "b"}, "cmd", "arg1", "arg2", {some: :opt}])
140+
// IO.popen({var: "a"}, ["cmd", "arg1", "arg2", {some: :opt}])
140141
shell = false and
141142
exists(ExprNodes::ArrayLiteralCfgNode arr | this.getArgument([0, 1]).asExpr() = arr |
142143
n = arr.getAnArgument()
143144
or
144-
// IO.popen({var: "a"}, [{var: "b"}, ["cmd", "argv0"], "arg1", "arg2", {some: :opt}])
145-
n = arr.getArgument(0).(ExprNodes::ArrayLiteralCfgNode).getArgument(0)
145+
// IO.popen([{var: "b"}, ["cmd", "argv0"], "arg1", "arg2", {some: :opt}])
146+
// IO.popen([["cmd", "argv0"], "arg1", "arg2", {some: :opt}])
147+
n = arr.getArgument([0, 1]).(ExprNodes::ArrayLiteralCfgNode).getArgument(0)
146148
)
147149
)
148150
)

‎ruby/ql/test/library-tests/frameworks/core/IO.expected

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,20 @@ ioPOpenCalls
88
| IO.rb:9:1:9:39 | call to popen |
99
| IO.rb:10:1:10:62 | call to popen |
1010
| IO.rb:11:1:11:76 | call to popen |
11-
| IO.rb:13:1:13:13 | call to popen |
12-
| IO.rb:14:1:14:36 | call to popen |
13-
| IO.rb:15:1:15:50 | call to popen |
14-
| IO.rb:18:1:18:13 | call to popen |
15-
| IO.rb:19:1:19:36 | call to popen |
16-
| IO.rb:20:1:20:50 | call to popen |
17-
| IO.rb:23:1:23:13 | call to popen |
18-
| IO.rb:24:1:24:36 | call to popen |
19-
| IO.rb:25:1:25:50 | call to popen |
20-
| IO.rb:28:1:28:13 | call to popen |
21-
| IO.rb:29:1:29:36 | call to popen |
22-
| IO.rb:30:1:30:50 | call to popen |
23-
| IO.rb:33:3:33:15 | call to popen |
11+
| IO.rb:12:1:12:76 | call to popen |
12+
| IO.rb:14:1:14:13 | call to popen |
13+
| IO.rb:15:1:15:36 | call to popen |
14+
| IO.rb:16:1:16:50 | call to popen |
15+
| IO.rb:19:1:19:13 | call to popen |
16+
| IO.rb:20:1:20:36 | call to popen |
17+
| IO.rb:21:1:21:50 | call to popen |
18+
| IO.rb:24:1:24:13 | call to popen |
19+
| IO.rb:25:1:25:36 | call to popen |
20+
| IO.rb:26:1:26:50 | call to popen |
21+
| IO.rb:29:1:29:13 | call to popen |
22+
| IO.rb:30:1:30:36 | call to popen |
23+
| IO.rb:31:1:31:50 | call to popen |
24+
| IO.rb:34:3:34:15 | call to popen |
2425
ioPOpenCallArguments
2526
| IO.rb:1:1:1:30 | call to popen | true | IO.rb:1:10:1:29 | "cat foo.txt \| tail" |
2627
| IO.rb:2:1:2:53 | call to popen | true | IO.rb:2:33:2:52 | "cat foo.txt \| tail" |
@@ -33,18 +34,22 @@ ioPOpenCallArguments
3334
| IO.rb:7:1:7:65 | call to popen | false | IO.rb:7:41:7:49 | "foo.txt" |
3435
| IO.rb:9:1:9:39 | call to popen | false | IO.rb:9:12:9:16 | "cat" |
3536
| IO.rb:9:1:9:39 | call to popen | false | IO.rb:9:29:9:37 | "foo.txt" |
37+
| IO.rb:10:1:10:62 | call to popen | false | IO.rb:10:35:10:39 | "cat" |
3638
| IO.rb:10:1:10:62 | call to popen | false | IO.rb:10:52:10:60 | "foo.txt" |
39+
| IO.rb:11:1:11:76 | call to popen | false | IO.rb:11:35:11:39 | "cat" |
3740
| IO.rb:11:1:11:76 | call to popen | false | IO.rb:11:52:11:60 | "foo.txt" |
38-
| IO.rb:13:1:13:13 | call to popen | false | IO.rb:13:10:13:12 | "-" |
39-
| IO.rb:14:1:14:36 | call to popen | false | IO.rb:14:33:14:35 | "-" |
40-
| IO.rb:15:1:15:50 | call to popen | false | IO.rb:15:33:15:35 | "-" |
41-
| IO.rb:18:1:18:13 | call to popen | true | IO.rb:18:10:18:12 | cmd |
42-
| IO.rb:19:1:19:36 | call to popen | true | IO.rb:19:33:19:35 | cmd |
43-
| IO.rb:20:1:20:50 | call to popen | true | IO.rb:20:33:20:35 | cmd |
44-
| IO.rb:23:1:23:13 | call to popen | true | IO.rb:23:10:23:12 | cmd |
45-
| IO.rb:24:1:24:36 | call to popen | true | IO.rb:24:33:24:35 | cmd |
46-
| IO.rb:25:1:25:50 | call to popen | true | IO.rb:25:33:25:35 | cmd |
47-
| IO.rb:28:1:28:13 | call to popen | true | IO.rb:28:10:28:12 | cmd |
48-
| IO.rb:29:1:29:36 | call to popen | true | IO.rb:29:33:29:35 | cmd |
49-
| IO.rb:30:1:30:50 | call to popen | true | IO.rb:30:33:30:35 | cmd |
50-
| IO.rb:33:3:33:15 | call to popen | true | IO.rb:33:12:33:14 | cmd |
41+
| IO.rb:12:1:12:76 | call to popen | false | IO.rb:12:35:12:39 | "cat" |
42+
| IO.rb:12:1:12:76 | call to popen | false | IO.rb:12:52:12:60 | "foo.txt" |
43+
| IO.rb:14:1:14:13 | call to popen | false | IO.rb:14:10:14:12 | "-" |
44+
| IO.rb:15:1:15:36 | call to popen | false | IO.rb:15:33:15:35 | "-" |
45+
| IO.rb:16:1:16:50 | call to popen | false | IO.rb:16:33:16:35 | "-" |
46+
| IO.rb:19:1:19:13 | call to popen | true | IO.rb:19:10:19:12 | cmd |
47+
| IO.rb:20:1:20:36 | call to popen | true | IO.rb:20:33:20:35 | cmd |
48+
| IO.rb:21:1:21:50 | call to popen | true | IO.rb:21:33:21:35 | cmd |
49+
| IO.rb:24:1:24:13 | call to popen | true | IO.rb:24:10:24:12 | cmd |
50+
| IO.rb:25:1:25:36 | call to popen | true | IO.rb:25:33:25:35 | cmd |
51+
| IO.rb:26:1:26:50 | call to popen | true | IO.rb:26:33:26:35 | cmd |
52+
| IO.rb:29:1:29:13 | call to popen | true | IO.rb:29:10:29:12 | cmd |
53+
| IO.rb:30:1:30:36 | call to popen | true | IO.rb:30:33:30:35 | cmd |
54+
| IO.rb:31:1:31:50 | call to popen | true | IO.rb:31:33:31:35 | cmd |
55+
| IO.rb:34:3:34:15 | call to popen | true | IO.rb:34:12:34:14 | cmd |

‎ruby/ql/test/library-tests/frameworks/core/IO.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
IO.popen([["cat", "argv0"], "foo.txt"])
1010
IO.popen([{some_env_var: "123"}, ["cat", "argv0"], "foo.txt"])
1111
IO.popen([{some_env_var: "123"}, ["cat", "argv0"], "foo.txt"], {some: :opt})
12+
IO.popen({some_env_var: "123"}, [["cat", "argv0"], "foo.txt"], {some: :opt})
1213

1314
IO.popen("-")
1415
IO.popen({some_env_var: "123"}, "-")

0 commit comments

Comments
 (0)