Skip to content

Commit 91bca4a

Browse files
committed
Ruby: limit ActiveRecord conditions sink to first array element
1 parent 2950890 commit 91bca4a

File tree

2 files changed

+10
-20
lines changed

2 files changed

+10
-20
lines changed

ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,14 @@ private predicate sqlFragmentArgumentInner(DataFlow::CallNode call, DataFlow::No
195195
or
196196
// This format was supported until Rails 2.3.8
197197
call = activeRecordQueryBuilderCall(["all", "find", "first", "last"]) and
198-
sink = call.getKeywordArgument("conditions")
198+
exists(DataFlow::LocalSourceNode sn |
199+
sn = call.getKeywordArgument("conditions").getALocalSource()
200+
|
201+
sink = sn.(DataFlow::ArrayLiteralNode).getElement(0)
202+
or
203+
sn.(DataFlow::LiteralNode).asLiteralAstNode() instanceof StringlikeLiteral and
204+
sink = sn
205+
)
199206
or
200207
call = activeRecordQueryBuilderCall("reload") and
201208
sink = call.getKeywordArgument("lock")

ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,8 @@
11
edges
22
| ActiveRecordInjection.rb:8:25:8:28 | name | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | provenance | AdditionalTaintStep |
33
| ActiveRecordInjection.rb:8:25:8:28 | name | ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | provenance | AdditionalTaintStep |
4-
| ActiveRecordInjection.rb:8:25:8:28 | name | ActiveRecordInjection.rb:14:56:14:59 | name | provenance | |
54
| ActiveRecordInjection.rb:8:31:8:34 | pass | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | provenance | AdditionalTaintStep |
65
| ActiveRecordInjection.rb:8:31:8:34 | pass | ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | provenance | AdditionalTaintStep |
7-
| ActiveRecordInjection.rb:8:31:8:34 | pass | ActiveRecordInjection.rb:14:62:14:65 | pass | provenance | |
8-
| ActiveRecordInjection.rb:12:30:12:66 | call to [] [element 0] | ActiveRecordInjection.rb:12:30:12:66 | call to [] | provenance | |
9-
| ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:12:30:12:66 | call to [] [element 0] | provenance | |
10-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 1] | ActiveRecordInjection.rb:14:30:14:66 | call to [] | provenance | |
11-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 2] | ActiveRecordInjection.rb:14:30:14:66 | call to [] | provenance | |
12-
| ActiveRecordInjection.rb:14:56:14:59 | name | ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 1] | provenance | |
13-
| ActiveRecordInjection.rb:14:62:14:65 | pass | ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 2] | provenance | |
146
| ActiveRecordInjection.rb:24:22:24:30 | condition | ActiveRecordInjection.rb:27:16:27:24 | condition | provenance | |
157
| ActiveRecordInjection.rb:39:30:39:35 | call to params | ActiveRecordInjection.rb:39:30:39:44 | ...[...] | provenance | |
168
| ActiveRecordInjection.rb:43:18:43:23 | call to params | ActiveRecordInjection.rb:43:18:43:32 | ...[...] | provenance | |
@@ -109,14 +101,7 @@ nodes
109101
| ActiveRecordInjection.rb:8:25:8:28 | name | semmle.label | name |
110102
| ActiveRecordInjection.rb:8:31:8:34 | pass | semmle.label | pass |
111103
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | semmle.label | "name='#{...}' and pass='#{...}'" |
112-
| ActiveRecordInjection.rb:12:30:12:66 | call to [] | semmle.label | call to [] |
113-
| ActiveRecordInjection.rb:12:30:12:66 | call to [] [element 0] | semmle.label | call to [] [element 0] |
114104
| ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | semmle.label | "name='#{...}' and pass='#{...}'" |
115-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] | semmle.label | call to [] |
116-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 1] | semmle.label | call to [] [element 1] |
117-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] [element 2] | semmle.label | call to [] [element 2] |
118-
| ActiveRecordInjection.rb:14:56:14:59 | name | semmle.label | name |
119-
| ActiveRecordInjection.rb:14:62:14:65 | pass | semmle.label | pass |
120105
| ActiveRecordInjection.rb:24:22:24:30 | condition | semmle.label | condition |
121106
| ActiveRecordInjection.rb:27:16:27:24 | condition | semmle.label | condition |
122107
| ActiveRecordInjection.rb:39:30:39:35 | call to params | semmle.label | call to params |
@@ -248,10 +233,8 @@ subpaths
248233
#select
249234
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:74:23:74:28 | call to params | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:23:74:28 | call to params | user-provided value |
250235
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:74:38:74:43 | call to params | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:38:74:43 | call to params | user-provided value |
251-
| ActiveRecordInjection.rb:12:30:12:66 | call to [] | ActiveRecordInjection.rb:74:23:74:28 | call to params | ActiveRecordInjection.rb:12:30:12:66 | call to [] | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:23:74:28 | call to params | user-provided value |
252-
| ActiveRecordInjection.rb:12:30:12:66 | call to [] | ActiveRecordInjection.rb:74:38:74:43 | call to params | ActiveRecordInjection.rb:12:30:12:66 | call to [] | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:38:74:43 | call to params | user-provided value |
253-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] | ActiveRecordInjection.rb:74:23:74:28 | call to params | ActiveRecordInjection.rb:14:30:14:66 | call to [] | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:23:74:28 | call to params | user-provided value |
254-
| ActiveRecordInjection.rb:14:30:14:66 | call to [] | ActiveRecordInjection.rb:74:38:74:43 | call to params | ActiveRecordInjection.rb:14:30:14:66 | call to [] | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:38:74:43 | call to params | user-provided value |
236+
| ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:74:23:74:28 | call to params | ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:23:74:28 | call to params | user-provided value |
237+
| ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:74:38:74:43 | call to params | ActiveRecordInjection.rb:12:31:12:65 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:74:38:74:43 | call to params | user-provided value |
255238
| ActiveRecordInjection.rb:27:16:27:24 | condition | ActiveRecordInjection.rb:171:21:171:26 | call to params | ActiveRecordInjection.rb:27:16:27:24 | condition | This SQL query depends on a $@. | ActiveRecordInjection.rb:171:21:171:26 | call to params | user-provided value |
256239
| ActiveRecordInjection.rb:39:30:39:44 | ...[...] | ActiveRecordInjection.rb:39:30:39:35 | call to params | ActiveRecordInjection.rb:39:30:39:44 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:39:30:39:35 | call to params | user-provided value |
257240
| ActiveRecordInjection.rb:43:18:43:32 | ...[...] | ActiveRecordInjection.rb:43:18:43:23 | call to params | ActiveRecordInjection.rb:43:18:43:32 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:43:18:43:23 | call to params | user-provided value |

0 commit comments

Comments
 (0)