Merge pull request github#11871 from hmac/rack

Author: hmac

ruby/ql/lib/change-notes/2023-01-12-rack.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Access to headers stored in the `env` of Rack requests is now recognized as a source of remote input.

ruby/ql/lib/codeql/ruby/Frameworks.qll

@@ -16,6 +16,7 @@ private import codeql.ruby.frameworks.ActiveSupport
 private import codeql.ruby.frameworks.Archive
 private import codeql.ruby.frameworks.Arel
 private import codeql.ruby.frameworks.GraphQL
+private import codeql.ruby.frameworks.Rack
 private import codeql.ruby.frameworks.Rails
 private import codeql.ruby.frameworks.Railties
 private import codeql.ruby.frameworks.Stdlib

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

@@ -1002,6 +1002,9 @@ class CallableNode extends ExprNode {
   /** Gets the `n`th positional parameter. */
   ParameterNode getParameter(int n) { this.getParameterPosition(result).isPositional(n) }
 
+  /** Gets the number of positional parameters of this callable. */
+  final int getNumberOfParameters() { result = count(this.getParameter(_)) }
+
   /** Gets the keyword parameter of the given name. */
   ParameterNode getKeywordParameter(string name) {
     this.getParameterPosition(result).isKeyword(name)

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

@@ -301,27 +301,39 @@ private module Request {
     override Http::Server::RequestInputKind getKind() { result = Http::Server::bodyInputKind() }
   }
 
-  /**
-   * A method call on `request` which returns the rack env.
-   * This is a hash containing all the information about the request. Values
-   * under keys starting with `HTTP_` are user-controlled.
-   */
-  private class EnvCall extends RequestMethodCall {
-    EnvCall() { this.getMethodName() = ["env", "filtered_env"] }
-  }
+  private module Env {
+    abstract private class Env extends DataFlow::LocalSourceNode { }
+
+    /**
+     * A method call on `request` which returns the rack env.
+     * This is a hash containing all the information about the request. Values
+     * under keys starting with `HTTP_` are user-controlled.
+     */
+    private class RequestEnvCall extends DataFlow::CallNode, Env {
+      RequestEnvCall() { this.getMethodName() = ["env", "filtered_env"] }
+    }
 
-  /**
-   * A read of a user-controlled parameter from the request env.
-   */
-  private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
-    EnvHttpAccess() {
-      this = any(EnvCall c).getAMethodCall("[]") and
-      this.getArgument(0).getConstantValue().getString().regexpMatch("^HTTP_.+")
+    private import codeql.ruby.frameworks.Rack
+
+    private class RackEnv extends Env {
+      RackEnv() { this = any(Rack::AppCandidate app).getEnv().getALocalUse() }
     }
 
-    override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
+    /**
+     * A read of a user-controlled parameter from the request env.
+     */
+    private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
+      EnvHttpAccess() {
+        this = any(Env c).getAMethodCall("[]") and
+        exists(string key | key = this.getArgument(0).getConstantValue().getString() |
+          key.regexpMatch("^HTTP_.+") or key = "PATH_INFO"
+        )
+      }
 
-    override string getSourceType() { result = "ActionDispatch::Request#env[]" }
+      override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
+
+      override string getSourceType() { result = "ActionDispatch::Request#env[]" }
+    }
   }
 }
 

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

@@ -0,0 +1,48 @@
+/**
+ * Provides modeling for the Rack library.
+ */
+
+private import codeql.ruby.controlflow.CfgNodes::ExprNodes
+private import codeql.ruby.DataFlow
+private import codeql.ruby.typetracking.TypeTracker
+
+/**
+ * Provides modeling for the Rack library.
+ */
+module Rack {
+  /**
+   * A class that may be a rack application.
+   * This is a class that has a `call` method that takes a single argument
+   * (traditionally called `env`) and returns a rack-compatible response.
+   */
+  class AppCandidate extends DataFlow::ClassNode {
+    private DataFlow::MethodNode call;
+
+    AppCandidate() {
+      call = this.getInstanceMethod("call") and
+      call.getNumberOfParameters() = 1 and
+      call.getReturn() = trackRackResponse()
+    }
+
+    /**
+     * Gets the environment of the request, which is the lone parameter to the `call` method.
+     */
+    DataFlow::ParameterNode getEnv() { result = call.getParameter(0) }
+  }
+
+  private predicate isRackResponse(DataFlow::Node r) {
+    // [status, headers, body]
+    r.asExpr().(ArrayLiteralCfgNode).getNumberOfArguments() = 3
+  }
+
+  private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t) {
+    t.start() and
+    isRackResponse(result)
+    or
+    exists(TypeTracker t2 | result = trackRackResponse(t2).track(t2, t))
+  }
+
+  private DataFlow::Node trackRackResponse() {
+    trackRackResponse(TypeTracker::end()).flowsTo(result)
+  }
+}

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

@@ -0,0 +1,4 @@
+| rack.rb:1:1:5:3 | HelloWorld | rack.rb:2:12:2:14 | env |
+| rack.rb:7:1:16:3 | Proxy | rack.rb:12:12:12:18 | the_env |
+| rack.rb:18:1:31:3 | Logger | rack.rb:24:12:24:14 | env |
+| rack.rb:45:1:61:3 | Baz | rack.rb:46:12:46:14 | env |

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

@@ -0,0 +1,4 @@
+private import codeql.ruby.frameworks.Rack
+private import codeql.ruby.DataFlow
+
+query predicate rackApps(Rack::AppCandidate c, DataFlow::ParameterNode env) { env = c.getEnv() }

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

@@ -0,0 +1,61 @@
+class HelloWorld
+  def call(env)
+    [200, {'Content-Type' => 'text/plain'}, ['Hello World']]
+  end
+end
+
+class Proxy
+  def initialize(app)
+    @app = app
+  end
+
+  def call(the_env)
+    status, headers, body = @app.call(the_env)
+    [status, headers, body]
+  end
+end
+
+class Logger
+  def initialize(app, logger = nil)
+    @app = app
+    @logger = logger
+  end
+
+  def call(env)
+    began_at = Utils.clock_time
+    status, header, body = @app.call(env)
+    header = Utils::HeaderHash.new(header)
+    body = BodyProxy.new(body) { log(env, status, header, began_at) }
+    [status, header, body]
+  end
+end
+
+class Foo
+  def not_call(env)
+    [1, 2, 3]
+  end
+end
+
+class Bar
+  def call(env)
+    nil
+  end
+end
+
+class Baz
+  def call(env)
+    run(env)
+  end
+
+  def run(env)
+    if env[:foo] == "foo"
+      [200, {}, "foo"]
+    else
+      error
+    end
+  end
+
+  def error
+    [400, {}, "nope"]
+  end
+end