Skip to content

Commit 3a1b294

Browse files
hmachmac
committed
Identify more ActiveRecord calculate methods
`average`, `count`, `maximum`, `minimum` and `sum` are all convenience methods that call `calculate(:<method name>, ...)` under the hood. Therefore they are vulnerable to SQL injection too.
1 parent 5219b1a commit 3a1b294

File tree

3 files changed

+46
-35
lines changed

3 files changed

+46
-35
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ private Expr sqlFragmentArgument(MethodCall call) {
7575
or
7676
methodName = "calculate" and result = call.getArgument(1)
7777
or
78+
methodName in ["average", "count", "maximum", "minimum", "sum"] and
79+
result = call.getArgument(0)
80+
or
7881
// This format was supported until Rails 2.3.8
7982
methodName = ["all", "find", "first", "last"] and
8083
result = call.getKeywordArgument("conditions")

‎ql/test/query-tests/security/cwe-089/ActiveRecordInjection.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ def some_request_handler
3434
# where `params[:column]` is unsanitized
3535
User.calculate(:average, params[:column])
3636

37+
# BAD: executes `SELECT MAX(#{params[:column]}) FROM "users"`
38+
# where `params[:column]` is unsanitized
39+
User.maximum(params[:column])
40+
3741
# BAD: executes `DELETE FROM "users" WHERE (id = '#{params[:id]}')`
3842
# where `params[:id]` is unsanitized
3943
User.delete_all("id = '#{params[:id]}'")

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

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,18 @@ edges
33
| ActiveRecordInjection.rb:8:31:8:34 | pass : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" |
44
| ActiveRecordInjection.rb:20:23:20:31 | condition : | ActiveRecordInjection.rb:23:17:23:25 | condition |
55
| ActiveRecordInjection.rb:35:30:35:35 | call to params : | ActiveRecordInjection.rb:35:30:35:44 | ...[...] |
6-
| ActiveRecordInjection.rb:39:30:39:35 | call to params : | ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" |
7-
| ActiveRecordInjection.rb:43:32:43:37 | call to params : | ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" |
8-
| ActiveRecordInjection.rb:48:21:48:26 | call to params : | ActiveRecordInjection.rb:47:16:47:21 | <<-SQL |
9-
| ActiveRecordInjection.rb:54:34:54:39 | call to params : | ActiveRecordInjection.rb:54:20:54:47 | "user.id = '#{...}'" |
10-
| ActiveRecordInjection.rb:56:23:56:28 | call to params : | ActiveRecordInjection.rb:56:23:56:35 | ...[...] : |
11-
| ActiveRecordInjection.rb:56:23:56:35 | ...[...] : | ActiveRecordInjection.rb:8:25:8:28 | name : |
12-
| ActiveRecordInjection.rb:56:38:56:43 | call to params : | ActiveRecordInjection.rb:56:38:56:50 | ...[...] : |
13-
| ActiveRecordInjection.rb:56:38:56:50 | ...[...] : | ActiveRecordInjection.rb:8:31:8:34 | pass : |
14-
| ActiveRecordInjection.rb:62:10:62:15 | call to params : | ActiveRecordInjection.rb:68:21:68:33 | ... + ... |
15-
| ActiveRecordInjection.rb:101:22:101:27 | call to params : | ActiveRecordInjection.rb:101:22:101:45 | ...[...] : |
16-
| ActiveRecordInjection.rb:101:22:101:45 | ...[...] : | ActiveRecordInjection.rb:20:23:20:31 | condition : |
6+
| ActiveRecordInjection.rb:39:18:39:23 | call to params : | ActiveRecordInjection.rb:39:18:39:32 | ...[...] |
7+
| ActiveRecordInjection.rb:43:30:43:35 | call to params : | ActiveRecordInjection.rb:43:21:43:43 | "id = '#{...}'" |
8+
| ActiveRecordInjection.rb:47:32:47:37 | call to params : | ActiveRecordInjection.rb:47:23:47:45 | "id = '#{...}'" |
9+
| ActiveRecordInjection.rb:52:21:52:26 | call to params : | ActiveRecordInjection.rb:51:16:51:21 | <<-SQL |
10+
| ActiveRecordInjection.rb:58:34:58:39 | call to params : | ActiveRecordInjection.rb:58:20:58:47 | "user.id = '#{...}'" |
11+
| ActiveRecordInjection.rb:60:23:60:28 | call to params : | ActiveRecordInjection.rb:60:23:60:35 | ...[...] : |
12+
| ActiveRecordInjection.rb:60:23:60:35 | ...[...] : | ActiveRecordInjection.rb:8:25:8:28 | name : |
13+
| ActiveRecordInjection.rb:60:38:60:43 | call to params : | ActiveRecordInjection.rb:60:38:60:50 | ...[...] : |
14+
| ActiveRecordInjection.rb:60:38:60:50 | ...[...] : | ActiveRecordInjection.rb:8:31:8:34 | pass : |
15+
| ActiveRecordInjection.rb:66:10:66:15 | call to params : | ActiveRecordInjection.rb:72:21:72:33 | ... + ... |
16+
| ActiveRecordInjection.rb:105:22:105:27 | call to params : | ActiveRecordInjection.rb:105:22:105:45 | ...[...] : |
17+
| ActiveRecordInjection.rb:105:22:105:45 | ...[...] : | ActiveRecordInjection.rb:20:23:20:31 | condition : |
1718
nodes
1819
| ActiveRecordInjection.rb:8:25:8:28 | name : | semmle.label | name : |
1920
| ActiveRecordInjection.rb:8:31:8:34 | pass : | semmle.label | pass : |
@@ -22,30 +23,33 @@ nodes
2223
| ActiveRecordInjection.rb:23:17:23:25 | condition | semmle.label | condition |
2324
| ActiveRecordInjection.rb:35:30:35:35 | call to params : | semmle.label | call to params : |
2425
| ActiveRecordInjection.rb:35:30:35:44 | ...[...] | semmle.label | ...[...] |
25-
| ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
26-
| ActiveRecordInjection.rb:39:30:39:35 | call to params : | semmle.label | call to params : |
27-
| ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
28-
| ActiveRecordInjection.rb:43:32:43:37 | call to params : | semmle.label | call to params : |
29-
| ActiveRecordInjection.rb:47:16:47:21 | <<-SQL | semmle.label | <<-SQL |
30-
| ActiveRecordInjection.rb:48:21:48:26 | call to params : | semmle.label | call to params : |
31-
| ActiveRecordInjection.rb:54:20:54:47 | "user.id = '#{...}'" | semmle.label | "user.id = '#{...}'" |
32-
| ActiveRecordInjection.rb:54:34:54:39 | call to params : | semmle.label | call to params : |
33-
| ActiveRecordInjection.rb:56:23:56:28 | call to params : | semmle.label | call to params : |
34-
| ActiveRecordInjection.rb:56:23:56:35 | ...[...] : | semmle.label | ...[...] : |
35-
| ActiveRecordInjection.rb:56:38:56:43 | call to params : | semmle.label | call to params : |
36-
| ActiveRecordInjection.rb:56:38:56:50 | ...[...] : | semmle.label | ...[...] : |
37-
| ActiveRecordInjection.rb:62:10:62:15 | call to params : | semmle.label | call to params : |
38-
| ActiveRecordInjection.rb:68:21:68:33 | ... + ... | semmle.label | ... + ... |
39-
| ActiveRecordInjection.rb:101:22:101:27 | call to params : | semmle.label | call to params : |
40-
| ActiveRecordInjection.rb:101:22:101:45 | ...[...] : | semmle.label | ...[...] : |
26+
| ActiveRecordInjection.rb:39:18:39:23 | call to params : | semmle.label | call to params : |
27+
| ActiveRecordInjection.rb:39:18:39:32 | ...[...] | semmle.label | ...[...] |
28+
| ActiveRecordInjection.rb:43:21:43:43 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
29+
| ActiveRecordInjection.rb:43:30:43:35 | call to params : | semmle.label | call to params : |
30+
| ActiveRecordInjection.rb:47:23:47:45 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
31+
| ActiveRecordInjection.rb:47:32:47:37 | call to params : | semmle.label | call to params : |
32+
| ActiveRecordInjection.rb:51:16:51:21 | <<-SQL | semmle.label | <<-SQL |
33+
| ActiveRecordInjection.rb:52:21:52:26 | call to params : | semmle.label | call to params : |
34+
| ActiveRecordInjection.rb:58:20:58:47 | "user.id = '#{...}'" | semmle.label | "user.id = '#{...}'" |
35+
| ActiveRecordInjection.rb:58:34:58:39 | call to params : | semmle.label | call to params : |
36+
| ActiveRecordInjection.rb:60:23:60:28 | call to params : | semmle.label | call to params : |
37+
| ActiveRecordInjection.rb:60:23:60:35 | ...[...] : | semmle.label | ...[...] : |
38+
| ActiveRecordInjection.rb:60:38:60:43 | call to params : | semmle.label | call to params : |
39+
| ActiveRecordInjection.rb:60:38:60:50 | ...[...] : | semmle.label | ...[...] : |
40+
| ActiveRecordInjection.rb:66:10:66:15 | call to params : | semmle.label | call to params : |
41+
| ActiveRecordInjection.rb:72:21:72:33 | ... + ... | semmle.label | ... + ... |
42+
| ActiveRecordInjection.rb:105:22:105:27 | call to params : | semmle.label | call to params : |
43+
| ActiveRecordInjection.rb:105:22:105:45 | ...[...] : | semmle.label | ...[...] : |
4144
subpaths
4245
#select
43-
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:56:23:56:28 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:56:23:56:28 | call to params | a user-provided value |
44-
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:56:38:56:43 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:56:38:56:43 | call to params | a user-provided value |
45-
| ActiveRecordInjection.rb:23:17:23:25 | condition | ActiveRecordInjection.rb:101:22:101:27 | call to params : | ActiveRecordInjection.rb:23:17:23:25 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:101:22:101:27 | call to params | a user-provided value |
46+
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:60:23:60:28 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:60:23:60:28 | call to params | a user-provided value |
47+
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:60:38:60:43 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:60:38:60:43 | call to params | a user-provided value |
48+
| ActiveRecordInjection.rb:23:17:23:25 | condition | ActiveRecordInjection.rb:105:22:105:27 | call to params : | ActiveRecordInjection.rb:23:17:23:25 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:105:22:105:27 | call to params | a user-provided value |
4649
| ActiveRecordInjection.rb:35:30:35:44 | ...[...] | ActiveRecordInjection.rb:35:30:35:35 | call to params : | ActiveRecordInjection.rb:35:30:35:44 | ...[...] | This SQL query depends on $@. | ActiveRecordInjection.rb:35:30:35:35 | call to params | a user-provided value |
47-
| ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" | ActiveRecordInjection.rb:39:30:39:35 | call to params : | ActiveRecordInjection.rb:39:21:39:43 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:39:30:39:35 | call to params | a user-provided value |
48-
| ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" | ActiveRecordInjection.rb:43:32:43:37 | call to params : | ActiveRecordInjection.rb:43:23:43:45 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:43:32:43:37 | call to params | a user-provided value |
49-
| ActiveRecordInjection.rb:47:16:47:21 | <<-SQL | ActiveRecordInjection.rb:48:21:48:26 | call to params : | ActiveRecordInjection.rb:47:16:47:21 | <<-SQL | This SQL query depends on $@. | ActiveRecordInjection.rb:48:21:48:26 | call to params | a user-provided value |
50-
| ActiveRecordInjection.rb:54:20:54:47 | "user.id = '#{...}'" | ActiveRecordInjection.rb:54:34:54:39 | call to params : | ActiveRecordInjection.rb:54:20:54:47 | "user.id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:54:34:54:39 | call to params | a user-provided value |
51-
| ActiveRecordInjection.rb:68:21:68:33 | ... + ... | ActiveRecordInjection.rb:62:10:62:15 | call to params : | ActiveRecordInjection.rb:68:21:68:33 | ... + ... | This SQL query depends on $@. | ActiveRecordInjection.rb:62:10:62:15 | call to params | a user-provided value |
50+
| ActiveRecordInjection.rb:39:18:39:32 | ...[...] | ActiveRecordInjection.rb:39:18:39:23 | call to params : | ActiveRecordInjection.rb:39:18:39:32 | ...[...] | This SQL query depends on $@. | ActiveRecordInjection.rb:39:18:39:23 | call to params | a user-provided value |
51+
| ActiveRecordInjection.rb:43:21:43:43 | "id = '#{...}'" | ActiveRecordInjection.rb:43:30:43:35 | call to params : | ActiveRecordInjection.rb:43:21:43:43 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:43:30:43:35 | call to params | a user-provided value |
52+
| ActiveRecordInjection.rb:47:23:47:45 | "id = '#{...}'" | ActiveRecordInjection.rb:47:32:47:37 | call to params : | ActiveRecordInjection.rb:47:23:47:45 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:47:32:47:37 | call to params | a user-provided value |
53+
| ActiveRecordInjection.rb:51:16:51:21 | <<-SQL | ActiveRecordInjection.rb:52:21:52:26 | call to params : | ActiveRecordInjection.rb:51:16:51:21 | <<-SQL | This SQL query depends on $@. | ActiveRecordInjection.rb:52:21:52:26 | call to params | a user-provided value |
54+
| ActiveRecordInjection.rb:58:20:58:47 | "user.id = '#{...}'" | ActiveRecordInjection.rb:58:34:58:39 | call to params : | ActiveRecordInjection.rb:58:20:58:47 | "user.id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:58:34:58:39 | call to params | a user-provided value |
55+
| ActiveRecordInjection.rb:72:21:72:33 | ... + ... | ActiveRecordInjection.rb:66:10:66:15 | call to params : | ActiveRecordInjection.rb:72:21:72:33 | ... + ... | This SQL query depends on $@. | ActiveRecordInjection.rb:66:10:66:15 | call to params | a user-provided value |

0 commit comments

Comments
 (0)