Ruby: Accept MaD sanitizers for queries with MaD sinks and convert some existing sanitizers

ruby/ql/lib/change-notes/2026-02-17-flow-through-shellwords-escape-shellescape.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* We now track taint flow through `Shellwords.escape` and `Shellwords.shellescape` for all queries except command injection, for which they are sanitizers.

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -9,6 +9,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.DataFlow
 private import codeql.ruby.dataflow.internal.DataFlowImplSpecific
 private import codeql.ruby.Frameworks
+private import codeql.ruby.frameworks.data.internal.ApiGraphModels
 private import codeql.ruby.dataflow.RemoteFlowSources
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.Regexp as RE
@@ -95,6 +96,10 @@ module SqlSanitization {
   abstract class Range extends DataFlow::Node { }
 }
 
+private class ExternalSqlInjectionSanitizer extends SqlSanitization::Range {
+  ExternalSqlInjectionSanitizer() { ModelOutput::barrierNode(this, "sql-injection") }
+}
+
 /**
  * A data-flow node that executes a regular expression.
  *

ruby/ql/lib/codeql/ruby/frameworks/Mysql2.model.yml

@@ -0,0 +1,11 @@
+extensions:
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: summaryModel
+    data:
+      - ['Mysql2::Client!', 'Method[escape]', 'Argument[0]', 'ReturnValue', 'taint']
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: barrierModel
+    data:
+      - ['Mysql2::Client!', 'Method[escape].ReturnValue', 'sql-injection']

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

@@ -48,26 +48,4 @@ module Mysql2 {
 
     override DataFlow::Node getSql() { result = query }
   }
-
-  /**
-   * A call to `Mysql2::Client.escape`, considered as a sanitizer for SQL statements.
-   */
-  private class Mysql2EscapeSanitization extends SqlSanitization::Range {
-    Mysql2EscapeSanitization() {
-      this = API::getTopLevelMember("Mysql2").getMember("Client").getAMethodCall("escape")
-    }
-  }
-
-  /**
-   * Flow summary for `Mysql2::Client.escape()`.
-   */
-  private class EscapeSummary extends SummarizedCallable::Range {
-    EscapeSummary() { this = "Mysql2::Client.escape()" }
-
-    override MethodCall getACall() { result = any(Mysql2EscapeSanitization c).asExpr().getExpr() }
-
-    override predicate propagatesFlow(string input, string output, boolean preservesValue) {
-      input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
-    }
-  }
 }

ruby/ql/lib/codeql/ruby/frameworks/Sqlite3.model.yml

@@ -0,0 +1,11 @@
+extensions:
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: summaryModel
+    data:
+      - ['SQLite3::Database!', 'Method[quote]', 'Argument[0]', 'ReturnValue', 'taint']
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: barrierModel
+    data:
+      - ['SQLite3::Database!', 'Method[quote].ReturnValue', 'sql-injection']

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

@@ -76,26 +76,4 @@ module Sqlite3 {
 
     override DataFlow::Node getSql() { result = this.getArgument(0) }
   }
-
-  /**
-   * A call to `SQLite3::Database.quote`, considered as a sanitizer for SQL statements.
-   */
-  private class SQLite3QuoteSanitization extends SqlSanitization {
-    SQLite3QuoteSanitization() {
-      this = API::getTopLevelMember("SQLite3").getMember("Database").getAMethodCall("quote")
-    }
-  }
-
-  /**
-   * Flow summary for `SQLite3::Database.quote()`.
-   */
-  private class QuoteSummary extends SummarizedCallable::Range {
-    QuoteSummary() { this = "SQLite3::Database.quote()" }
-
-    override MethodCall getACall() { result = any(SQLite3QuoteSanitization c).asExpr().getExpr() }
-
-    override predicate propagatesFlow(string input, string output, boolean preservesValue) {
-      input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
-    }
-  }
 }

ruby/ql/lib/codeql/ruby/frameworks/stdlib/Shellwords.model.yml

@@ -0,0 +1,12 @@
+extensions:
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: summaryModel
+    data:
+      - ['Shellwords!', 'Method[escape,shellescape]', 'Argument[0]', 'ReturnValue', 'taint']
+
+  - addsTo:
+      pack: codeql/ruby-all
+      extensible: barrierModel
+    data:
+      - ['Shellwords!', 'Method[escape,shellescape].ReturnValue', 'command-injection']

ruby/ql/lib/codeql/ruby/security/CodeInjectionCustomizations.qll

@@ -118,4 +118,8 @@ module CodeInjection {
   private class ExternalCodeInjectionSink extends Sink {
     ExternalCodeInjectionSink() { ModelOutput::sinkNode(this, "code-injection") }
   }
+
+  private class ExternalCodeInjectionSanitizer extends Sanitizer {
+    ExternalCodeInjectionSanitizer() { ModelOutput::barrierNode(this, "code-injection") }
+  }
 }

ruby/ql/lib/codeql/ruby/security/CommandInjectionCustomizations.qll

@@ -43,18 +43,20 @@ module CommandInjection {
   }
 
   /**
-   * A call to `Shellwords.escape` or `Shellwords.shellescape` sanitizes its input.
+   * A call to `String#shellescape` sanitizes its input.
    */
   class ShellwordsEscapeAsSanitizer extends Sanitizer {
     ShellwordsEscapeAsSanitizer() {
-      this = API::getTopLevelMember("Shellwords").getAMethodCall(["escape", "shellescape"])
-      or
-      // The method is also added as `String#shellescape`.
+      // The `Shellwords.shellescape` method is also added as `String#shellescape`.
       this.(DataFlow::CallNode).getMethodName() = "shellescape"
     }
   }
 
   private class ExternalCommandInjectionSink extends Sink {
     ExternalCommandInjectionSink() { ModelOutput::sinkNode(this, "command-injection") }
   }
+
+  private class ExternalCommandInjectionSanitizer extends Sanitizer {
+    ExternalCommandInjectionSanitizer() { ModelOutput::barrierNode(this, "command-injection") }
+  }
 }

ruby/ql/lib/codeql/ruby/security/LogInjectionQuery.qll

@@ -67,6 +67,10 @@ class HtmlEscapingAsSanitizer extends Sanitizer {
   HtmlEscapingAsSanitizer() { this = any(HtmlEscaping esc).getOutput() }
 }
 
+private class ExternalLogInjectionSanitizer extends Sanitizer {
+  ExternalLogInjectionSanitizer() { ModelOutput::barrierNode(this, "log-injection") }
+}
+
 private module LogInjectionConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 

ruby/ql/lib/codeql/ruby/security/PathInjectionCustomizations.qll

@@ -57,4 +57,8 @@ module PathInjection {
   private class ExternalPathInjectionSink extends Sink {
     ExternalPathInjectionSink() { ModelOutput::sinkNode(this, "path-injection") }
   }
+
+  private class ExternalPathInjectionSanitizer extends Sanitizer {
+    ExternalPathInjectionSanitizer() { ModelOutput::barrierNode(this, "path-injection") }
+  }
 }

ruby/ql/lib/codeql/ruby/security/ServerSideRequestForgeryCustomizations.qll

@@ -46,4 +46,8 @@ module ServerSideRequestForgery {
   private class ExternalRequestForgerySink extends Sink {
     ExternalRequestForgerySink() { ModelOutput::sinkNode(this, "request-forgery") }
   }
+
+  private class ExternalRequestForgerySanitizer extends Sanitizer {
+    ExternalRequestForgerySanitizer() { ModelOutput::barrierNode(this, "request-forgery") }
+  }
 }

ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll

@@ -125,6 +125,10 @@ module UrlRedirect {
    */
   class StringInterpolationAsSanitizer extends PrefixedStringInterpolation, Sanitizer { }
 
+  private class ExternalUrlRedirectSanitizer extends Sanitizer {
+    ExternalUrlRedirectSanitizer() { ModelOutput::barrierNode(this, "url-redirection") }
+  }
+
   /**
    * These methods return a new `ActionController::Parameters` or a `Hash` containing a subset of
    * the original values. This may still contain user input, so the results are tainted.

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

@@ -2921,13 +2921,11 @@
 | file://:0:0:0:0 | [summary param] position 0 in File.realdirpath | file://:0:0:0:0 | [summary] to write: ReturnValue in File.realdirpath |
 | file://:0:0:0:0 | [summary param] position 0 in File.realpath | file://:0:0:0:0 | [summary] to write: ReturnValue in File.realpath |
 | file://:0:0:0:0 | [summary param] position 0 in Hash[] | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in Hash[] |
-| file://:0:0:0:0 | [summary param] position 0 in Mysql2::Client.escape() | file://:0:0:0:0 | [summary] to write: ReturnValue in Mysql2::Client.escape() |
 | file://:0:0:0:0 | [summary param] position 0 in Mysql2::Client.new() | file://:0:0:0:0 | [summary] to write: ReturnValue in Mysql2::Client.new() |
 | file://:0:0:0:0 | [summary param] position 0 in Net::LDAP.new | file://:0:0:0:0 | [summary] to write: ReturnValue in Net::LDAP.new |
 | file://:0:0:0:0 | [summary param] position 0 in Net::LDAP::Filter | file://:0:0:0:0 | [summary] to write: ReturnValue in Net::LDAP::Filter |
 | file://:0:0:0:0 | [summary param] position 0 in PG.new() | file://:0:0:0:0 | [summary] to write: ReturnValue in PG.new() |
 | file://:0:0:0:0 | [summary param] position 0 in Rack::Utils.parse_query | file://:0:0:0:0 | [summary] to write: ReturnValue in Rack::Utils.parse_query |
-| file://:0:0:0:0 | [summary param] position 0 in SQLite3::Database.quote() | file://:0:0:0:0 | [summary] to write: ReturnValue in SQLite3::Database.quote() |
 | file://:0:0:0:0 | [summary param] position 0 in Sequel.connect | file://:0:0:0:0 | [summary] to write: ReturnValue in Sequel.connect |
 | file://:0:0:0:0 | [summary param] position 0 in String.try_convert | file://:0:0:0:0 | [summary] to write: ReturnValue in String.try_convert |
 | file://:0:0:0:0 | [summary param] position 0 in \| | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in \| |

ruby/ql/test/library-tests/frameworks/mysql2/Mysql2.rb

@@ -1,6 +1,6 @@
 class UsersController < ActionController::Base
   def mysql2_handler(event:, context:)
-    name = params[:user_name]
+    name = params[:user_name] # $ Source[rb/sql-injection]
 
     conn = Mysql2::Client.new(
         host: "127.0.0.1",
@@ -10,7 +10,7 @@ def mysql2_handler(event:, context:)
     results1 = conn.query("SELECT * FROM users")
 
     # BAD: SQL statement constructed from user input
-    results2 = conn.query("SELECT * FROM users WHERE username='#{name}'")
+    results2 = conn.query("SELECT * FROM users WHERE username='#{name}'") # $ Alert[rb/sql-injection]
 
     # GOOD: user input is escaped
     escaped = Mysql2::Client.escape(name)
@@ -21,10 +21,10 @@ def mysql2_handler(event:, context:)
     results4 = statement1.execute(1, name, :as => :array)
 
     # BAD: SQL statement constructed from user input
-    statement2 = conn.prepare("SELECT * FROM users WHERE username='#{name}' AND password = ?")
+    statement2 = conn.prepare("SELECT * FROM users WHERE username='#{name}' AND password = ?") # $ Alert[rb/sql-injection]
     results4 = statement2.execute("password", :as => :array)
 
     # NOT EXECUTED
     statement3 = conn.prepare("SELECT * FROM users WHERE username = ?")
   end
-end
+end

ruby/ql/test/library-tests/frameworks/mysql2/SqlInjection.expected

@@ -0,0 +1,15 @@
+#select
+| Mysql2.rb:13:27:13:72 | "SELECT * FROM users WHERE use..." | Mysql2.rb:3:12:3:17 | call to params | Mysql2.rb:13:27:13:72 | "SELECT * FROM users WHERE use..." | This SQL query depends on a $@. | Mysql2.rb:3:12:3:17 | call to params | user-provided value |
+| Mysql2.rb:24:31:24:93 | "SELECT * FROM users WHERE use..." | Mysql2.rb:3:12:3:17 | call to params | Mysql2.rb:24:31:24:93 | "SELECT * FROM users WHERE use..." | This SQL query depends on a $@. | Mysql2.rb:3:12:3:17 | call to params | user-provided value |
+edges
+| Mysql2.rb:3:5:3:8 | name | Mysql2.rb:13:27:13:72 | "SELECT * FROM users WHERE use..." | provenance | AdditionalTaintStep |
+| Mysql2.rb:3:5:3:8 | name | Mysql2.rb:24:31:24:93 | "SELECT * FROM users WHERE use..." | provenance | AdditionalTaintStep |
+| Mysql2.rb:3:12:3:17 | call to params | Mysql2.rb:3:12:3:29 | ...[...] | provenance |  |
+| Mysql2.rb:3:12:3:29 | ...[...] | Mysql2.rb:3:5:3:8 | name | provenance |  |
+nodes
+| Mysql2.rb:3:5:3:8 | name | semmle.label | name |
+| Mysql2.rb:3:12:3:17 | call to params | semmle.label | call to params |
+| Mysql2.rb:3:12:3:29 | ...[...] | semmle.label | ...[...] |
+| Mysql2.rb:13:27:13:72 | "SELECT * FROM users WHERE use..." | semmle.label | "SELECT * FROM users WHERE use..." |
+| Mysql2.rb:24:31:24:93 | "SELECT * FROM users WHERE use..." | semmle.label | "SELECT * FROM users WHERE use..." |
+subpaths

ruby/ql/test/library-tests/frameworks/mysql2/SqlInjection.qlref

@@ -0,0 +1,4 @@
+query: queries/security/cwe-089/SqlInjection.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

ruby/ql/test/library-tests/frameworks/sqlite3/SqlInjection.expected

@@ -0,0 +1,12 @@
+#select
+| sqlite3.rb:29:16:29:67 | "select * from table where cat..." | sqlite3.rb:25:16:25:21 | call to params | sqlite3.rb:29:16:29:67 | "select * from table where cat..." | This SQL query depends on a $@. | sqlite3.rb:25:16:25:21 | call to params | user-provided value |
+edges
+| sqlite3.rb:25:5:25:12 | category | sqlite3.rb:29:16:29:67 | "select * from table where cat..." | provenance | AdditionalTaintStep |
+| sqlite3.rb:25:16:25:21 | call to params | sqlite3.rb:25:16:25:32 | ...[...] | provenance |  |
+| sqlite3.rb:25:16:25:32 | ...[...] | sqlite3.rb:25:5:25:12 | category | provenance |  |
+nodes
+| sqlite3.rb:25:5:25:12 | category | semmle.label | category |
+| sqlite3.rb:25:16:25:21 | call to params | semmle.label | call to params |
+| sqlite3.rb:25:16:25:32 | ...[...] | semmle.label | ...[...] |
+| sqlite3.rb:29:16:29:67 | "select * from table where cat..." | semmle.label | "select * from table where cat..." |
+subpaths

ruby/ql/test/library-tests/frameworks/sqlite3/SqlInjection.qlref

@@ -0,0 +1,4 @@
+query: queries/security/cwe-089/SqlInjection.ql
+postprocess:
+  - utils/test/PrettyPrintModels.ql
+  - utils/test/InlineExpectationsTestQuery.ql

ruby/ql/test/library-tests/frameworks/sqlite3/Sqlite3.expected

@@ -2,9 +2,11 @@ sqlite3SqlConstruction
 | sqlite3.rb:5:1:5:17 | call to execute | sqlite3.rb:5:12:5:17 | <<-SQL |
 | sqlite3.rb:12:8:12:41 | call to prepare | sqlite3.rb:12:19:12:41 | "select * from numbers" |
 | sqlite3.rb:17:3:19:5 | call to execute | sqlite3.rb:17:15:17:35 | "select * from table" |
-| sqlite3.rb:29:7:29:40 | call to execute | sqlite3.rb:29:19:29:39 | "select * from table" |
+| sqlite3.rb:29:5:29:68 | call to execute | sqlite3.rb:29:16:29:67 | "select * from table where cat..." |
+| sqlite3.rb:33:5:33:78 | call to execute | sqlite3.rb:33:16:33:77 | "select * from table where cat..." |
 sqlite3SqlExecution
 | sqlite3.rb:5:1:5:17 | call to execute | sqlite3.rb:5:12:5:17 | <<-SQL |
 | sqlite3.rb:14:1:14:12 | call to execute | sqlite3.rb:12:19:12:41 | "select * from numbers" |
 | sqlite3.rb:17:3:19:5 | call to execute | sqlite3.rb:17:15:17:35 | "select * from table" |
-| sqlite3.rb:29:7:29:40 | call to execute | sqlite3.rb:29:19:29:39 | "select * from table" |
+| sqlite3.rb:29:5:29:68 | call to execute | sqlite3.rb:29:16:29:67 | "select * from table where cat..." |
+| sqlite3.rb:33:5:33:78 | call to execute | sqlite3.rb:33:16:33:77 | "select * from table where cat..." |

ruby/ql/test/library-tests/frameworks/sqlite3/sqlite3.rb

@@ -20,12 +20,16 @@
 end
 
 
-class MyDatabaseWrapper
-      def initialize(filename)
-      @db = SQLite3::Database.new(filename, results_as_hash: true)
-    end
-
-    def select_rows(category)
-      @db.execute("select * from table")
-    end
+class SqliteController < ActionController::Base
+  def sqlite3_handler
+    category = params[:category] # $ Source[rb/sql-injection]
+    db = SQLite3::Database.new "test.db"
+
+    # BAD: SQL injection vulnerability
+    db.execute("select * from table where category = '#{category}'") # $ Alert[rb/sql-injection]
+
+    # GOOD: Sanitized by SQLite3::Database.quote
+    sanitized_category = SQLite3::Database.quote(category)
+    db.execute("select * from table where category = '#{sanitized_category}'")
+  end
 end