Skip to content

Commit 270d13e

Browse files
hmachmac
committed
Identify more vulnerable ActiveRecord methods
`find_by!`, `find_or_create_by`, `find_or_create_by!` and `find_or_initialize_by` act similarly to `find_by`.
1 parent 56919ee commit 270d13e

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ private Expr sqlFragmentArgument(MethodCall call) {
6868
(
6969
methodName =
7070
[
71-
"delete_by", "destroy_by", "exists?", "find_by", "find_by_sql", "from", "group", "having",
71+
"delete_by", "destroy_by", "exists?", "find_by", "find_by!", "find_or_create_by",
72+
"find_or_create_by!", "find_or_initialize_by", "find_by_sql", "from", "group", "having",
7273
"joins", "lock", "not", "order", "pluck", "where"
7374
] and
7475
result = call.getArgument(0)

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ def some_request_handler
5858
User.where.not("user.id = '#{params[:id]}'")
5959

6060
User.authenticate(params[:name], params[:pass])
61+
62+
# BAD: executes `SELECT "users".* FROM "users" WHERE (id = '#{params[:id]}')` LIMIT 1
63+
# where `params[:id]` is unsanitized
64+
User.find_or_initialize_by("id = '#{params[:id]}'")
6165
end
6266
end
6367

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ edges
1212
| ActiveRecordInjection.rb:60:23:60:35 | ...[...] : | ActiveRecordInjection.rb:8:25:8:28 | name : |
1313
| ActiveRecordInjection.rb:60:38:60:43 | call to params : | ActiveRecordInjection.rb:60:38:60:50 | ...[...] : |
1414
| 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:20:72:32 | ... + ... |
16-
| ActiveRecordInjection.rb:105:21:105:26 | call to params : | ActiveRecordInjection.rb:105:21:105:44 | ...[...] : |
17-
| ActiveRecordInjection.rb:105:21:105:44 | ...[...] : | ActiveRecordInjection.rb:20:22:20:30 | condition : |
15+
| ActiveRecordInjection.rb:64:41:64:46 | call to params : | ActiveRecordInjection.rb:64:32:64:54 | "id = '#{...}'" |
16+
| ActiveRecordInjection.rb:70:10:70:15 | call to params : | ActiveRecordInjection.rb:76:20:76:32 | ... + ... |
17+
| ActiveRecordInjection.rb:109:21:109:26 | call to params : | ActiveRecordInjection.rb:109:21:109:44 | ...[...] : |
18+
| ActiveRecordInjection.rb:109:21:109:44 | ...[...] : | ActiveRecordInjection.rb:20:22:20:30 | condition : |
1819
nodes
1920
| ActiveRecordInjection.rb:8:25:8:28 | name : | semmle.label | name : |
2021
| ActiveRecordInjection.rb:8:31:8:34 | pass : | semmle.label | pass : |
@@ -37,19 +38,22 @@ nodes
3738
| ActiveRecordInjection.rb:60:23:60:35 | ...[...] : | semmle.label | ...[...] : |
3839
| ActiveRecordInjection.rb:60:38:60:43 | call to params : | semmle.label | call to params : |
3940
| 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:20:72:32 | ... + ... | semmle.label | ... + ... |
42-
| ActiveRecordInjection.rb:105:21:105:26 | call to params : | semmle.label | call to params : |
43-
| ActiveRecordInjection.rb:105:21:105:44 | ...[...] : | semmle.label | ...[...] : |
41+
| ActiveRecordInjection.rb:64:32:64:54 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
42+
| ActiveRecordInjection.rb:64:41:64:46 | call to params : | semmle.label | call to params : |
43+
| ActiveRecordInjection.rb:70:10:70:15 | call to params : | semmle.label | call to params : |
44+
| ActiveRecordInjection.rb:76:20:76:32 | ... + ... | semmle.label | ... + ... |
45+
| ActiveRecordInjection.rb:109:21:109:26 | call to params : | semmle.label | call to params : |
46+
| ActiveRecordInjection.rb:109:21:109:44 | ...[...] : | semmle.label | ...[...] : |
4447
subpaths
4548
#select
4649
| 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 |
4750
| 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:16:23:24 | condition | ActiveRecordInjection.rb:105:21:105:26 | call to params : | ActiveRecordInjection.rb:23:16:23:24 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:105:21:105:26 | call to params | a user-provided value |
51+
| ActiveRecordInjection.rb:23:16:23:24 | condition | ActiveRecordInjection.rb:109:21:109:26 | call to params : | ActiveRecordInjection.rb:23:16:23:24 | condition | This SQL query depends on $@. | ActiveRecordInjection.rb:109:21:109:26 | call to params | a user-provided value |
4952
| 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 |
5053
| 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 |
5154
| ActiveRecordInjection.rb:43:20:43:42 | "id = '#{...}'" | ActiveRecordInjection.rb:43:29:43:34 | call to params : | ActiveRecordInjection.rb:43:20:43:42 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:43:29:43:34 | call to params | a user-provided value |
5255
| ActiveRecordInjection.rb:47:22:47:44 | "id = '#{...}'" | ActiveRecordInjection.rb:47:31:47:36 | call to params : | ActiveRecordInjection.rb:47:22:47:44 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:47:31:47:36 | call to params | a user-provided value |
5356
| 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 |
5457
| 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:20:72:32 | ... + ... | ActiveRecordInjection.rb:66:10:66:15 | call to params : | ActiveRecordInjection.rb:72:20:72:32 | ... + ... | This SQL query depends on $@. | ActiveRecordInjection.rb:66:10:66:15 | call to params | a user-provided value |
58+
| ActiveRecordInjection.rb:64:32:64:54 | "id = '#{...}'" | ActiveRecordInjection.rb:64:41:64:46 | call to params : | ActiveRecordInjection.rb:64:32:64:54 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:64:41:64:46 | call to params | a user-provided value |
59+
| ActiveRecordInjection.rb:76:20:76:32 | ... + ... | ActiveRecordInjection.rb:70:10:70:15 | call to params : | ActiveRecordInjection.rb:76:20:76:32 | ... + ... | This SQL query depends on $@. | ActiveRecordInjection.rb:70:10:70:15 | call to params | a user-provided value |

0 commit comments

Comments
 (0)