Skip to content

Commit 7191e1c

Browse files
committed
Re-add delete_all and destroy_all methods
These methods don't take any arguments in Rails versions > 3, but there's no harm in checking for them anyway, and some people might be using very old Rails versions.
1 parent 75bbc51 commit 7191e1c

File tree

3 files changed

+20
-12
lines changed

3 files changed

+20
-12
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ private Expr sqlFragmentArgument(MethodCall call) {
6868
(
6969
methodName =
7070
[
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",
73-
"joins", "lock", "not", "order", "pluck", "where", "rewhere", "select", "reselect",
74-
"update_all"
71+
"delete_all", "delete_by", "destroy_all", "destroy_by", "exists?", "find_by", "find_by!",
72+
"find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "from",
73+
"group", "having", "joins", "lock", "not", "order", "pluck", "where", "rewhere", "select",
74+
"reselect", "update_all"
7575
] and
7676
result = call.getArgument(0)
7777
or

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,19 @@ def some_request_handler
4242
# where `params[:id]` is unsanitized
4343
User.delete_by("id = '#{params[:id]}'")
4444

45-
46-
47-
48-
45+
# BAD: executes `DELETE FROM "users" WHERE (id = '#{params[:id]}')`
46+
# where `params[:id]` is unsanitized
47+
# (in Rails < 4.0)
48+
User.delete_all("id = '#{params[:id]}'")
4949

5050
# BAD: executes `SELECT "users".* FROM "users" WHERE (id = '#{params[:id]}')`
5151
# where `params[:id]` is unsanitized
5252
User.destroy_by(["id = '#{params[:id]}'"])
5353

54-
55-
56-
57-
54+
# BAD: executes `SELECT "users".* FROM "users" WHERE (id = '#{params[:id]}')`
55+
# where `params[:id]` is unsanitized
56+
# (in Rails < 4.0)
57+
User.destroy_all(["id = '#{params[:id]}'"])
5858

5959
# BAD: executes `SELECT "users".* FROM "users" WHERE id BETWEEN '#{params[:min_id]}' AND 100000`
6060
# where `params[:min_id]` is unsanitized

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ edges
55
| ActiveRecordInjection.rb:35:30:35:35 | call to params : | ActiveRecordInjection.rb:35:30:35:44 | ...[...] |
66
| ActiveRecordInjection.rb:39:18:39:23 | call to params : | ActiveRecordInjection.rb:39:18:39:32 | ...[...] |
77
| ActiveRecordInjection.rb:43:29:43:34 | call to params : | ActiveRecordInjection.rb:43:20:43:42 | "id = '#{...}'" |
8+
| ActiveRecordInjection.rb:48:30:48:35 | call to params : | ActiveRecordInjection.rb:48:21:48:43 | "id = '#{...}'" |
89
| ActiveRecordInjection.rb:52:31:52:36 | call to params : | ActiveRecordInjection.rb:52:22:52:44 | "id = '#{...}'" |
10+
| ActiveRecordInjection.rb:57:32:57:37 | call to params : | ActiveRecordInjection.rb:57:23:57:45 | "id = '#{...}'" |
911
| ActiveRecordInjection.rb:62:21:62:26 | call to params : | ActiveRecordInjection.rb:61:16:61:21 | <<-SQL |
1012
| ActiveRecordInjection.rb:68:34:68:39 | call to params : | ActiveRecordInjection.rb:68:20:68:47 | "user.id = '#{...}'" |
1113
| ActiveRecordInjection.rb:70:23:70:28 | call to params : | ActiveRecordInjection.rb:70:23:70:35 | ...[...] : |
@@ -32,8 +34,12 @@ nodes
3234
| ActiveRecordInjection.rb:39:18:39:32 | ...[...] | semmle.label | ...[...] |
3335
| ActiveRecordInjection.rb:43:20:43:42 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
3436
| ActiveRecordInjection.rb:43:29:43:34 | call to params : | semmle.label | call to params : |
37+
| ActiveRecordInjection.rb:48:21:48:43 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
38+
| ActiveRecordInjection.rb:48:30:48:35 | call to params : | semmle.label | call to params : |
3539
| ActiveRecordInjection.rb:52:22:52:44 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
3640
| ActiveRecordInjection.rb:52:31:52:36 | call to params : | semmle.label | call to params : |
41+
| ActiveRecordInjection.rb:57:23:57:45 | "id = '#{...}'" | semmle.label | "id = '#{...}'" |
42+
| ActiveRecordInjection.rb:57:32:57:37 | call to params : | semmle.label | call to params : |
3743
| ActiveRecordInjection.rb:61:16:61:21 | <<-SQL | semmle.label | <<-SQL |
3844
| ActiveRecordInjection.rb:62:21:62:26 | call to params : | semmle.label | call to params : |
3945
| ActiveRecordInjection.rb:68:20:68:47 | "user.id = '#{...}'" | semmle.label | "user.id = '#{...}'" |
@@ -64,7 +70,9 @@ subpaths
6470
| 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 |
6571
| 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 |
6672
| 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 |
73+
| ActiveRecordInjection.rb:48:21:48:43 | "id = '#{...}'" | ActiveRecordInjection.rb:48:30:48:35 | call to params : | ActiveRecordInjection.rb:48:21:48:43 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:48:30:48:35 | call to params | a user-provided value |
6774
| ActiveRecordInjection.rb:52:22:52:44 | "id = '#{...}'" | ActiveRecordInjection.rb:52:31:52:36 | call to params : | ActiveRecordInjection.rb:52:22:52:44 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:52:31:52:36 | call to params | a user-provided value |
75+
| ActiveRecordInjection.rb:57:23:57:45 | "id = '#{...}'" | ActiveRecordInjection.rb:57:32:57:37 | call to params : | ActiveRecordInjection.rb:57:23:57:45 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:57:32:57:37 | call to params | a user-provided value |
6876
| ActiveRecordInjection.rb:61:16:61:21 | <<-SQL | ActiveRecordInjection.rb:62:21:62:26 | call to params : | ActiveRecordInjection.rb:61:16:61:21 | <<-SQL | This SQL query depends on $@. | ActiveRecordInjection.rb:62:21:62:26 | call to params | a user-provided value |
6977
| ActiveRecordInjection.rb:68:20:68:47 | "user.id = '#{...}'" | ActiveRecordInjection.rb:68:34:68:39 | call to params : | ActiveRecordInjection.rb:68:20:68:47 | "user.id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:68:34:68:39 | call to params | a user-provided value |
7078
| ActiveRecordInjection.rb:74:32:74:54 | "id = '#{...}'" | ActiveRecordInjection.rb:74:41:74:46 | call to params : | ActiveRecordInjection.rb:74:32:74:54 | "id = '#{...}'" | This SQL query depends on $@. | ActiveRecordInjection.rb:74:41:74:46 | call to params | a user-provided value |

0 commit comments

Comments
 (0)