Skip to content

Commit

Permalink
Merge pull request github#15857 from joefarebrother/ruby-activerecord…
Browse files Browse the repository at this point in the history
…-from

Ruby: Model second argument of `ActiveRecord` `from`
  • Loading branch information
joefarebrother authored Mar 11, 2024
2 parents 0443620 + dbd33d1 commit 9c51514
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 68 deletions.
4 changes: 4 additions & 0 deletions ruby/ql/lib/change-notes/2024-03-08-activerecord-from.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The second argument, `subquery_name`, of the `ActiveRecord::QueryMethods::from` method, is now recognized as an sql injection sink.
6 changes: 3 additions & 3 deletions ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ private predicate sqlFragmentArgumentInner(DataFlow::CallNode call, DataFlow::No
call =
activeRecordQueryBuilderCall([
"delete_all", "delete_by", "destroy_all", "destroy_by", "exists?", "find_by", "find_by!",
"find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "from",
"having", "lock", "not", "where", "rewhere"
"find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "having",
"lock", "not", "where", "rewhere"
]) and
sink = call.getArgument(0)
or
call =
activeRecordQueryBuilderCall([
"group", "joins", "order", "reorder", "pluck", "select", "reselect"
"from", "group", "joins", "order", "reorder", "pluck", "select", "reselect"
]) and
sink = call.getArgument(_)
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ def some_request_handler
User.joins(:a, params[:column])

User.count_by_sql(params[:custom_sql_query])

# BAD: executes `SELECT users.* FROM #{params[:tab]}`
# where `params[:tab]` is unsanitized
User.all.from(params[:tab])
# BAD: executes `SELECT "users".* FROM (SELECT "users".* FROM "users") #{params[:sq]}
User.all.from(User.all, params[:sq])
end
end

Expand Down
Loading

0 comments on commit 9c51514

Please sign in to comment.