Skip to content

Commit c97dccf

Browse files
hmachmac
committed
Ruby: Add reorder as a SQL sink
In recent versions of Rails this method doesn't seem to be vulnerable, but it may be in previous versions. There's a slight FP risk here, but I think it is small.
1 parent ab58d4c commit c97dccf

File tree

3 files changed

+27
-21
lines changed

3 files changed

+27
-21
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ private Expr sqlFragmentArgument(MethodCall call) {
116116
[
117117
"delete_all", "delete_by", "destroy_all", "destroy_by", "exists?", "find_by", "find_by!",
118118
"find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "from",
119-
"group", "having", "joins", "lock", "not", "order", "pluck", "where", "rewhere", "select",
120-
"reselect", "update_all"
119+
"group", "having", "joins", "lock", "not", "order", "reorder", "pluck", "where",
120+
"rewhere", "select", "reselect", "update_all"
121121
] and
122122
result = call.getArgument(0)
123123
or

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ def some_request_handler
9090
# BAD: executes `UPDATE "users" SET #{params[:fields]}`
9191
# where `params[:fields]` is unsanitized
9292
User.update_all(params[:fields])
93+
94+
User.reorder(params[:direction])
9395
end
9496
end
9597

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

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ edges
2626
| ActiveRecordInjection.rb:84:19:84:24 | call to params : | ActiveRecordInjection.rb:84:19:84:33 | ...[...] |
2727
| ActiveRecordInjection.rb:88:18:88:23 | call to params : | ActiveRecordInjection.rb:88:18:88:35 | ...[...] |
2828
| ActiveRecordInjection.rb:92:21:92:26 | call to params : | ActiveRecordInjection.rb:92:21:92:35 | ...[...] |
29-
| ActiveRecordInjection.rb:98:10:98:15 | call to params : | ActiveRecordInjection.rb:99:11:99:12 | ps : |
30-
| ActiveRecordInjection.rb:99:11:99:12 | ps : | ActiveRecordInjection.rb:99:11:99:17 | ...[...] : |
31-
| ActiveRecordInjection.rb:99:11:99:17 | ...[...] : | ActiveRecordInjection.rb:104:20:104:32 | ... + ... |
32-
| ActiveRecordInjection.rb:137:21:137:26 | call to params : | ActiveRecordInjection.rb:137:21:137:44 | ...[...] : |
33-
| ActiveRecordInjection.rb:137:21:137:44 | ...[...] : | ActiveRecordInjection.rb:20:22:20:30 | condition : |
34-
| ActiveRecordInjection.rb:151:59:151:64 | call to params : | ActiveRecordInjection.rb:151:59:151:74 | ...[...] : |
35-
| ActiveRecordInjection.rb:151:59:151:74 | ...[...] : | ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." |
29+
| ActiveRecordInjection.rb:94:18:94:23 | call to params : | ActiveRecordInjection.rb:94:18:94:35 | ...[...] |
30+
| ActiveRecordInjection.rb:100:10:100:15 | call to params : | ActiveRecordInjection.rb:101:11:101:12 | ps : |
31+
| ActiveRecordInjection.rb:101:11:101:12 | ps : | ActiveRecordInjection.rb:101:11:101:17 | ...[...] : |
32+
| ActiveRecordInjection.rb:101:11:101:17 | ...[...] : | ActiveRecordInjection.rb:106:20:106:32 | ... + ... |
33+
| ActiveRecordInjection.rb:139:21:139:26 | call to params : | ActiveRecordInjection.rb:139:21:139:44 | ...[...] : |
34+
| ActiveRecordInjection.rb:139:21:139:44 | ...[...] : | ActiveRecordInjection.rb:20:22:20:30 | condition : |
35+
| ActiveRecordInjection.rb:153:59:153:64 | call to params : | ActiveRecordInjection.rb:153:59:153:74 | ...[...] : |
36+
| ActiveRecordInjection.rb:153:59:153:74 | ...[...] : | ActiveRecordInjection.rb:153:27:153:76 | "this is an unsafe annotation:..." |
3637
| ArelInjection.rb:4:12:4:17 | call to params : | ArelInjection.rb:4:12:4:29 | ...[...] : |
3738
| ArelInjection.rb:4:12:4:29 | ...[...] : | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." |
3839
nodes
@@ -78,23 +79,25 @@ nodes
7879
| ActiveRecordInjection.rb:88:18:88:35 | ...[...] | semmle.label | ...[...] |
7980
| ActiveRecordInjection.rb:92:21:92:26 | call to params : | semmle.label | call to params : |
8081
| ActiveRecordInjection.rb:92:21:92:35 | ...[...] | semmle.label | ...[...] |
81-
| ActiveRecordInjection.rb:98:10:98:15 | call to params : | semmle.label | call to params : |
82-
| ActiveRecordInjection.rb:99:11:99:12 | ps : | semmle.label | ps : |
83-
| ActiveRecordInjection.rb:99:11:99:17 | ...[...] : | semmle.label | ...[...] : |
84-
| ActiveRecordInjection.rb:104:20:104:32 | ... + ... | semmle.label | ... + ... |
85-
| ActiveRecordInjection.rb:137:21:137:26 | call to params : | semmle.label | call to params : |
86-
| ActiveRecordInjection.rb:137:21:137:44 | ...[...] : | semmle.label | ...[...] : |
87-
| ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | semmle.label | "this is an unsafe annotation:..." |
88-
| ActiveRecordInjection.rb:151:59:151:64 | call to params : | semmle.label | call to params : |
89-
| ActiveRecordInjection.rb:151:59:151:74 | ...[...] : | semmle.label | ...[...] : |
82+
| ActiveRecordInjection.rb:94:18:94:23 | call to params : | semmle.label | call to params : |
83+
| ActiveRecordInjection.rb:94:18:94:35 | ...[...] | semmle.label | ...[...] |
84+
| ActiveRecordInjection.rb:100:10:100:15 | call to params : | semmle.label | call to params : |
85+
| ActiveRecordInjection.rb:101:11:101:12 | ps : | semmle.label | ps : |
86+
| ActiveRecordInjection.rb:101:11:101:17 | ...[...] : | semmle.label | ...[...] : |
87+
| ActiveRecordInjection.rb:106:20:106:32 | ... + ... | semmle.label | ... + ... |
88+
| ActiveRecordInjection.rb:139:21:139:26 | call to params : | semmle.label | call to params : |
89+
| ActiveRecordInjection.rb:139:21:139:44 | ...[...] : | semmle.label | ...[...] : |
90+
| ActiveRecordInjection.rb:153:27:153:76 | "this is an unsafe annotation:..." | semmle.label | "this is an unsafe annotation:..." |
91+
| ActiveRecordInjection.rb:153:59:153:64 | call to params : | semmle.label | call to params : |
92+
| ActiveRecordInjection.rb:153:59:153:74 | ...[...] : | semmle.label | ...[...] : |
9093
| ArelInjection.rb:4:12:4:17 | call to params : | semmle.label | call to params : |
9194
| ArelInjection.rb:4:12:4:29 | ...[...] : | semmle.label | ...[...] : |
9295
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
9396
subpaths
9497
#select
9598
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:70:23:70:28 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:70:23:70:28 | call to params | user-provided value |
9699
| ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | ActiveRecordInjection.rb:70:38:70:43 | call to params : | ActiveRecordInjection.rb:10:33:10:67 | "name='#{...}' and pass='#{...}'" | This SQL query depends on a $@. | ActiveRecordInjection.rb:70:38:70:43 | call to params | user-provided value |
97-
| ActiveRecordInjection.rb:23:16:23:24 | condition | ActiveRecordInjection.rb:137:21:137:26 | call to params : | ActiveRecordInjection.rb:23:16:23:24 | condition | This SQL query depends on a $@. | ActiveRecordInjection.rb:137:21:137:26 | call to params | user-provided value |
100+
| ActiveRecordInjection.rb:23:16:23:24 | condition | ActiveRecordInjection.rb:139:21:139:26 | call to params : | ActiveRecordInjection.rb:23:16:23:24 | condition | This SQL query depends on a $@. | ActiveRecordInjection.rb:139:21:139:26 | call to params | user-provided value |
98101
| 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 a $@. | ActiveRecordInjection.rb:35:30:35:35 | call to params | user-provided value |
99102
| 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 a $@. | ActiveRecordInjection.rb:39:18:39:23 | call to params | user-provided value |
100103
| 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 a $@. | ActiveRecordInjection.rb:43:29:43:34 | call to params | user-provided value |
@@ -108,6 +111,7 @@ subpaths
108111
| ActiveRecordInjection.rb:84:19:84:33 | ...[...] | ActiveRecordInjection.rb:84:19:84:24 | call to params : | ActiveRecordInjection.rb:84:19:84:33 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:84:19:84:24 | call to params | user-provided value |
109112
| ActiveRecordInjection.rb:88:18:88:35 | ...[...] | ActiveRecordInjection.rb:88:18:88:23 | call to params : | ActiveRecordInjection.rb:88:18:88:35 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:88:18:88:23 | call to params | user-provided value |
110113
| ActiveRecordInjection.rb:92:21:92:35 | ...[...] | ActiveRecordInjection.rb:92:21:92:26 | call to params : | ActiveRecordInjection.rb:92:21:92:35 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:92:21:92:26 | call to params | user-provided value |
111-
| ActiveRecordInjection.rb:104:20:104:32 | ... + ... | ActiveRecordInjection.rb:98:10:98:15 | call to params : | ActiveRecordInjection.rb:104:20:104:32 | ... + ... | This SQL query depends on a $@. | ActiveRecordInjection.rb:98:10:98:15 | call to params | user-provided value |
112-
| ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | ActiveRecordInjection.rb:151:59:151:64 | call to params : | ActiveRecordInjection.rb:151:27:151:76 | "this is an unsafe annotation:..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:151:59:151:64 | call to params | user-provided value |
114+
| ActiveRecordInjection.rb:94:18:94:35 | ...[...] | ActiveRecordInjection.rb:94:18:94:23 | call to params : | ActiveRecordInjection.rb:94:18:94:35 | ...[...] | This SQL query depends on a $@. | ActiveRecordInjection.rb:94:18:94:23 | call to params | user-provided value |
115+
| ActiveRecordInjection.rb:106:20:106:32 | ... + ... | ActiveRecordInjection.rb:100:10:100:15 | call to params : | ActiveRecordInjection.rb:106:20:106:32 | ... + ... | This SQL query depends on a $@. | ActiveRecordInjection.rb:100:10:100:15 | call to params | user-provided value |
116+
| ActiveRecordInjection.rb:153:27:153:76 | "this is an unsafe annotation:..." | ActiveRecordInjection.rb:153:59:153:64 | call to params : | ActiveRecordInjection.rb:153:27:153:76 | "this is an unsafe annotation:..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:153:59:153:64 | call to params | user-provided value |
113117
| ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params : | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |

0 commit comments

Comments
 (0)