Ruby: ActiveRecord::Connection.execute SQL sink

Author: hmac

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

@@ -31,6 +31,18 @@ private predicate isBuiltInMethodForActiveRecordModelInstance(string methodName)
   methodName = objectInstanceMethodName()
 }
 
+private API::Node activeRecordClassApiNode() {
+  result =
+    // class Foo < ActiveRecord::Base
+    // class Bar < Foo
+    [
+      API::getTopLevelMember("ActiveRecord").getMember("Base"),
+      // In Rails applications `ApplicationRecord` typically extends `ActiveRecord::Base`, but we
+      // treat it separately in case the `ApplicationRecord` definition is not in the database.
+      API::getTopLevelMember("ApplicationRecord")
+    ].getASubclass()
+}
+
 /**
  * A `ClassDeclaration` for a class that inherits from `ActiveRecord::Base`. For example,
  *
@@ -45,15 +57,8 @@ private predicate isBuiltInMethodForActiveRecordModelInstance(string methodName)
  */
 class ActiveRecordModelClass extends ClassDeclaration {
   ActiveRecordModelClass() {
-    // class Foo < ActiveRecord::Base
-    // class Bar < Foo
     this.getSuperclassExpr() =
-      [
-        API::getTopLevelMember("ActiveRecord").getMember("Base"),
-        // In Rails applications `ApplicationRecord` typically extends `ActiveRecord::Base`, but we
-        // treat it separately in case the `ApplicationRecord` definition is not in the database.
-        API::getTopLevelMember("ApplicationRecord")
-      ].getASubclass().getAValueReachableFromSource().asExpr().getExpr()
+      activeRecordClassApiNode().getAValueReachableFromSource().asExpr().getExpr()
   }
 
   // Gets the class declaration for this class and all of its super classes
@@ -208,11 +213,22 @@ class ActiveRecordSqlExecutionRange extends SqlExecution::Range {
     exists(PotentiallyUnsafeSqlExecutingMethodCall mc |
       this.asExpr().getNode() = mc.getSqlFragmentSinkArgument()
     )
+    or
+    exists(DataFlow::CallNode executeCall |
+      executeCall.getReceiver() = activeRecordConnectionInstance() and
+      executeCall.getMethodName() = "execute" and
+      this = executeCall.getArgument(0) and
+      unsafeSqlExpr(this.asExpr().getExpr())
+    )
   }
 
   override DataFlow::Node getSql() { result = this }
 }
 
+private DataFlow::Node activeRecordConnectionInstance() {
+  result = activeRecordClassApiNode().getAMethodCall("connection")
+}
+
 // TODO: model `ActiveRecord` sanitizers
 // https://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html
 /**

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

@@ -172,4 +172,9 @@ def index
   def permitted_params
     params.require(:my_key).permit(:id, :user_id, :my_type)
   end
+  
+  def show
+    ActiveRecord::Base.connection.execute("SELECT * FROM users WHERE id = #{permitted_params[:user_id]}")
+    Regression.connection.execute("SELECT * FROM users WHERE id = #{permitted_params[:user_id]}")
+  end
 end

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

@@ -41,6 +41,12 @@ edges
 | ActiveRecordInjection.rb:173:5:173:10 | call to params :  | ActiveRecordInjection.rb:173:5:173:27 | call to require :  |
 | ActiveRecordInjection.rb:173:5:173:27 | call to require :  | ActiveRecordInjection.rb:173:5:173:59 | call to permit :  |
 | ActiveRecordInjection.rb:173:5:173:59 | call to permit :  | ActiveRecordInjection.rb:166:17:166:32 | call to permitted_params :  |
+| ActiveRecordInjection.rb:173:5:173:59 | call to permit :  | ActiveRecordInjection.rb:177:77:177:92 | call to permitted_params :  |
+| ActiveRecordInjection.rb:173:5:173:59 | call to permit :  | ActiveRecordInjection.rb:178:69:178:84 | call to permitted_params :  |
+| ActiveRecordInjection.rb:177:77:177:92 | call to permitted_params :  | ActiveRecordInjection.rb:177:77:177:102 | ...[...] :  |
+| ActiveRecordInjection.rb:177:77:177:102 | ...[...] :  | ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." |
+| ActiveRecordInjection.rb:178:69:178:84 | call to permitted_params :  | ActiveRecordInjection.rb:178:69:178:94 | ...[...] :  |
+| ActiveRecordInjection.rb:178:69:178:94 | ...[...] :  | ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." |
 | ArelInjection.rb:4:12:4:17 | call to params :  | ArelInjection.rb:4:12:4:29 | ...[...] :  |
 | ArelInjection.rb:4:12:4:29 | ...[...] :  | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." |
 nodes
@@ -106,6 +112,12 @@ nodes
 | ActiveRecordInjection.rb:173:5:173:10 | call to params :  | semmle.label | call to params :  |
 | ActiveRecordInjection.rb:173:5:173:27 | call to require :  | semmle.label | call to require :  |
 | ActiveRecordInjection.rb:173:5:173:59 | call to permit :  | semmle.label | call to permit :  |
+| ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." | semmle.label | "SELECT * FROM users WHERE id ..." |
+| ActiveRecordInjection.rb:177:77:177:92 | call to permitted_params :  | semmle.label | call to permitted_params :  |
+| ActiveRecordInjection.rb:177:77:177:102 | ...[...] :  | semmle.label | ...[...] :  |
+| ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." | semmle.label | "SELECT * FROM users WHERE id ..." |
+| ActiveRecordInjection.rb:178:69:178:84 | call to permitted_params :  | semmle.label | call to permitted_params :  |
+| ActiveRecordInjection.rb:178:69:178:94 | ...[...] :  | semmle.label | ...[...] :  |
 | ArelInjection.rb:4:12:4:17 | call to params :  | semmle.label | call to params :  |
 | ArelInjection.rb:4:12:4:29 | ...[...] :  | semmle.label | ...[...] :  |
 | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
@@ -132,4 +144,6 @@ subpaths
 | ActiveRecordInjection.rb:108:20:108:32 | ... + ... | ActiveRecordInjection.rb:102:10:102:15 | call to params :  | ActiveRecordInjection.rb:108:20:108:32 | ... + ... | This SQL query depends on a $@. | ActiveRecordInjection.rb:102:10:102:15 | call to params | user-provided value |
 | ActiveRecordInjection.rb:155:27:155:76 | "this is an unsafe annotation:..." | ActiveRecordInjection.rb:155:59:155:64 | call to params :  | ActiveRecordInjection.rb:155:27:155:76 | "this is an unsafe annotation:..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:155:59:155:64 | call to params | user-provided value |
 | ActiveRecordInjection.rb:168:37:168:41 | query | ActiveRecordInjection.rb:173:5:173:10 | call to params :  | ActiveRecordInjection.rb:168:37:168:41 | query | This SQL query depends on a $@. | ActiveRecordInjection.rb:173:5:173:10 | call to params | user-provided value |
+| ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:173:5:173:10 | call to params :  | ActiveRecordInjection.rb:177:43:177:104 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:173:5:173:10 | call to params | user-provided value |
+| ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:173:5:173:10 | call to params :  | ActiveRecordInjection.rb:178:35:178:96 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:173:5:173:10 | call to params | user-provided value |
 | 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 |