Merge pull request #13566 from alexrford/rb/rack-params

Ruby: add `Rack::Request` params and cookies as remote input sources

Author: alexrford

ruby/ql/lib/change-notes/2023-07-05-rack-response.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+* Query parameters and cookies from `Rack::Response` objects are recognized as potential sources of remote flow input.
+* Calls to `Rack::Utils.parse_query` now propagate taint.

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

@@ -7,7 +7,9 @@
  */
 module Rack {
   import rack.internal.App
+  import rack.internal.Request
   import rack.internal.Response::Public as Response
+  import rack.internal.Utils
 
   /** DEPRECATED: Alias for App::AppCandidate */
   deprecated class AppCandidate = App::AppCandidate;

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/Request.qll

@@ -0,0 +1,39 @@
+/**
+ * Provides modeling for the `Request` component of the `Rack` library.
+ */
+
+private import codeql.ruby.AST
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow
+
+/**
+ * Provides modeling for the `Request` component of the `Rack` library.
+ */
+module Request {
+  private class RackRequest extends API::Node {
+    RackRequest() { this = API::getTopLevelMember("Rack").getMember("Request").getInstance() }
+  }
+
+  /** An access to the parameters of a request to a rack application via a `Rack::Request` instance. */
+  private class RackRequestParamsAccess extends Http::Server::RequestInputAccess::Range {
+    RackRequestParamsAccess() {
+      this = any(RackRequest req).getAMethodCall(["params", "query_string", "[]", "fullpath"])
+    }
+
+    override string getSourceType() { result = "Rack::Request#params" }
+
+    override Http::Server::RequestInputKind getKind() {
+      result = Http::Server::parameterInputKind()
+    }
+  }
+
+  /** An access to the cookies of a request to a rack application via a `Rack::Request` instance. */
+  private class RackRequestCookiesAccess extends Http::Server::RequestInputAccess::Range {
+    RackRequestCookiesAccess() { this = any(RackRequest req).getAMethodCall("cookies") }
+
+    override string getSourceType() { result = "Rack::Request#cookies" }
+
+    override Http::Server::RequestInputKind getKind() { result = Http::Server::cookieInputKind() }
+  }
+}

ruby/ql/lib/codeql/ruby/frameworks/rack/internal/Utils.qll

@@ -0,0 +1,29 @@
+/**
+ * Provides modeling for the `Utils` component of the `Rack` library.
+ */
+
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.dataflow.FlowSummary
+
+/**
+ * Provides modeling for the `Utils` component of the `Rack` library.
+ */
+module Utils {
+  /** Flow summary for `Rack::Utils.parse_query`, which parses a query string. */
+  private class ParseQuerySummary extends SummarizedCallable {
+    ParseQuerySummary() { this = "Rack::Utils.parse_query" }
+
+    override MethodCall getACall() {
+      result =
+        API::getTopLevelMember("Rack")
+            .getMember("Utils")
+            .getAMethodCall("parse_query")
+            .asExpr()
+            .getExpr()
+    }
+
+    override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+      input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
+    }
+  }
+}

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

@@ -2816,6 +2816,7 @@
 | 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 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 |

ruby/ql/test/library-tests/frameworks/rack/Rack.expected

@@ -6,6 +6,8 @@ rackRequestHandlers
 | rack.rb:60:3:62:5 | call | rack.rb:60:12:60:14 | env | rack.rb:66:7:66:22 | call to [] |
 | rack.rb:60:3:62:5 | call | rack.rb:60:12:60:14 | env | rack.rb:73:5:73:21 | call to [] |
 | rack.rb:79:3:81:5 | call | rack.rb:79:17:79:19 | env | rack.rb:93:5:93:78 | call to finish |
+| rack.rb:98:3:107:5 | call | rack.rb:98:12:98:14 | env | rack.rb:110:5:110:28 | call to [] |
+| rack.rb:98:3:107:5 | call | rack.rb:98:12:98:14 | env | rack.rb:114:5:114:30 | call to [] |
 | rack_apps.rb:6:3:12:5 | call | rack_apps.rb:6:12:6:14 | env | rack_apps.rb:10:12:10:34 | call to [] |
 | rack_apps.rb:16:3:18:5 | call | rack_apps.rb:16:17:16:19 | env | rack_apps.rb:17:5:17:28 | call to [] |
 | rack_apps.rb:21:14:21:50 | -> { ... } | rack_apps.rb:21:17:21:19 | env | rack_apps.rb:21:24:21:48 | call to [] |
@@ -16,3 +18,7 @@ rackResponseContentTypes
 redirectResponses
 | rack.rb:43:5:43:45 | call to [] | rack.rb:42:30:42:40 | "/foo.html" |
 | rack.rb:93:5:93:78 | call to finish | rack.rb:93:60:93:70 | redirect_to |
+requestInputAccesses
+| rack.rb:100:18:100:28 | call to cookies |
+| rack.rb:103:14:103:23 | call to params |
+| rack.rb:104:18:104:32 | ...[...] |

ruby/ql/test/library-tests/frameworks/rack/Rack.ql

@@ -1,4 +1,5 @@
 private import codeql.ruby.AST
+private import codeql.ruby.Concepts
 private import codeql.ruby.frameworks.Rack
 private import codeql.ruby.DataFlow
 
@@ -17,3 +18,5 @@ query predicate rackResponseContentTypes(
 query predicate redirectResponses(Rack::Response::RedirectResponse resp, DataFlow::Node location) {
   location = resp.getRedirectLocation()
 }
+
+query predicate requestInputAccesses(Http::Server::RequestInputAccess ria) { any() }

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

@@ -93,3 +93,24 @@ def do_redirect
     Rack::Response.new(['redirecting'], 302, 'Location' => redirect_to).finish
   end
 end
+
+class UsesRequest
+  def call(env)
+    req = Rack::Request.new(env)
+    if session = req.cookies['session']
+      reuse_session(session)
+    else
+      name = req.params['name']
+      password = req['password']
+      login(name, password)
+    end
+  end
+
+  def login(name, password)
+    [200, {}, "new session"]
+  end
+
+  def reuse_session(name, password)
+    [200, {}, "reuse session"]
+  end
+end