Merge pull request #15718 from joefarebrother/ruby-arel-sqlliteral

Ruby: Model Arel::Nodes::SqlLiteral.new

Author: joefarebrother

ruby/ql/lib/change-notes/2024-02-26-arel-sqlliteral.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+Calls to `Arel::Nodes::SqlLiteral.new` are now modeled as instances of the `SqlConstruction` concept, as well as propagating taint from their argument.

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

@@ -39,4 +39,34 @@ module Arel {
 
     override DataFlow::Node getSql() { result = this.getArgument(0) }
   }
+
+  /**
+   * Flow summary for `Arel::Nodes::SqlLiteral.new`. This method wraps a SQL string, marking it as
+   * safe.
+   */
+  private class SqlLiteralNewSummary extends SummarizedCallable {
+    SqlLiteralNewSummary() { this = "Arel::Nodes::SqlLiteral.new" }
+
+    override MethodCall getACall() {
+      result = any(ArelSqlLiteralNewConstruction c).asExpr().getExpr()
+    }
+
+    override predicate propagatesFlow(string input, string output, boolean preservesValue) {
+      input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
+    }
+  }
+
+  /** A call to `Arel::Nodes::SqlLiteral.new`, considered as a SQL construction. */
+  private class ArelSqlLiteralNewConstruction extends SqlConstruction::Range, DataFlow::CallNode {
+    ArelSqlLiteralNewConstruction() {
+      this.asExpr() =
+        API::getTopLevelMember("Arel")
+            .getMember("Nodes")
+            .getMember("SqlLiteral")
+            .getAMethodCall("new")
+            .asExpr()
+    }
+
+    override DataFlow::Node getSql() { result = this.getArgument(0) }
+  }
 }

ruby/ql/test/library-tests/dataflow/local/TaintStep.expected

@@ -2804,6 +2804,7 @@
 | file://:0:0:0:0 | [summary param] position 0 in ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: Argument[self] in ActionController::Parameters#merge! |
 | file://:0:0:0:0 | [summary param] position 0 in ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: ReturnValue in ActionController::Parameters#merge! |
 | file://:0:0:0:0 | [summary param] position 0 in Arel.sql | file://:0:0:0:0 | [summary] to write: ReturnValue in Arel.sql |
+| file://:0:0:0:0 | [summary param] position 0 in Arel::Nodes::SqlLiteral.new | file://:0:0:0:0 | [summary] to write: ReturnValue in Arel::Nodes::SqlLiteral.new |
 | file://:0:0:0:0 | [summary param] position 0 in Base64.decode64() | file://:0:0:0:0 | [summary] to write: ReturnValue in Base64.decode64() |
 | file://:0:0:0:0 | [summary param] position 0 in ERB.new | file://:0:0:0:0 | [summary] to write: ReturnValue in ERB.new |
 | file://:0:0:0:0 | [summary param] position 0 in File.absolute_path | file://:0:0:0:0 | [summary] to write: ReturnValue in File.absolute_path |

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

@@ -4,5 +4,6 @@ def unsafe_action
     name = params[:user_name]
     # BAD: SQL statement constructed from user input
     sql = Arel.sql("SELECT * FROM users WHERE name = #{name}")
+    sql = Arel::Nodes::SqlLiteral.new("SELECT * FROM users WHERE name = #{name}")
   end
 end

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

@@ -73,6 +73,7 @@ edges
 | ActiveRecordInjection.rb:198:69:198:84 | call to permitted_params | ActiveRecordInjection.rb:198:69:198:94 | ...[...] | provenance |  |
 | ActiveRecordInjection.rb:198:69:198:94 | ...[...] | ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | provenance |  |
 | ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | provenance |  |
+| ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | provenance |  |
 | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:4:12:4:29 | ...[...] | provenance |  |
 | ArelInjection.rb:4:12:4:29 | ...[...] | ArelInjection.rb:4:5:4:8 | name | provenance |  |
 | PgInjection.rb:6:5:6:8 | name | PgInjection.rb:13:5:13:8 | qry1 | provenance |  |
@@ -194,6 +195,7 @@ nodes
 | 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..." |
+| ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | semmle.label | "SELECT * FROM users WHERE nam..." |
 | PgInjection.rb:6:5:6:8 | name | semmle.label | name |
 | PgInjection.rb:6:12:6:17 | call to params | semmle.label | call to params |
 | PgInjection.rb:6:12:6:24 | ...[...] | semmle.label | ...[...] |
@@ -244,6 +246,7 @@ subpaths
 | ActiveRecordInjection.rb:197:43:197:104 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:193:5:193:10 | call to params | ActiveRecordInjection.rb:197:43:197:104 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:193:5:193:10 | call to params | user-provided value |
 | ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:193:5:193:10 | call to params | ActiveRecordInjection.rb:198:35:198:96 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:193:5:193: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 |
+| ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |
 | PgInjection.rb:14:15:14:18 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:14:15:14:18 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
 | PgInjection.rb:15:21:15:24 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:15:21:15:24 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |
 | PgInjection.rb:20:22:20:25 | qry2 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:20:22:20:25 | qry2 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |