Merge pull request #7015 from github/hmac/ssrf

Ruby: Add Server-Side Request Forgery query

Author: hmac

ruby/change-notes/2021-11-09-request-forgery.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* A new query (`rb/request-forgery`) has been added. The query finds HTTP requests made with user-controlled URLs.

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

@@ -417,6 +417,12 @@ module HTTP {
       /** Gets a node which returns the body of the response */
       DataFlow::Node getResponseBody() { result = super.getResponseBody() }
 
+      /**
+       * Gets a node that contributes to the URL of the request.
+       * Depending on the framework, a request may have multiple nodes which contribute to the URL.
+       */
+      DataFlow::Node getURL() { result = super.getURL() }
+
       /** Gets a string that identifies the framework used for this request. */
       string getFramework() { result = super.getFramework() }
 
@@ -442,6 +448,12 @@ module HTTP {
         /** Gets a node which returns the body of the response */
         abstract DataFlow::Node getResponseBody();
 
+        /**
+         * Gets a node that contributes to the URL of the request.
+         * Depending on the framework, a request may have multiple nodes which contribute to the URL.
+         */
+        abstract DataFlow::Node getURL();
+
         /** Gets a string that identifies the framework used for this request. */
         abstract string getFramework();
 

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -320,7 +320,11 @@ module ExprNodes {
 
     /** Gets the the keyword argument whose key is `keyword` of this call. */
     final ExprCfgNode getKeywordArgument(string keyword) {
-      e.hasCfgChild(e.getKeywordArgument(keyword), this, result)
+      exists(PairCfgNode n |
+        e.hasCfgChild(e.getAnArgument(), this, n) and
+        n.getKey().getExpr().(SymbolLiteral).getValueText() = keyword and
+        result = n.getValue()
+      )
     }
 
     /** Gets the number of arguments of this call. */
@@ -426,6 +430,27 @@ module ExprNodes {
     ParenthesizedExprCfgNode() { this.getExpr() instanceof ParenthesizedExpr }
   }
 
+  private class PairChildMapping extends ExprChildMapping, Pair {
+    override predicate relevantChild(Expr e) { e = this.getKey() or e = this.getValue() }
+  }
+
+  /** A control-flow node that wraps a `Pair` AST expression. */
+  class PairCfgNode extends ExprCfgNode {
+    override PairChildMapping e;
+
+    final override Pair getExpr() { result = ExprCfgNode.super.getExpr() }
+
+    /**
+     * Gets the key expression of this pair.
+     */
+    final ExprCfgNode getKey() { e.hasCfgChild(e.getKey(), this, result) }
+
+    /**
+     * Gets the value expression of this pair.
+     */
+    final ExprCfgNode getValue() { e.hasCfgChild(e.getValue(), this, result) }
+  }
+
   /** A control-flow node that wraps a `VariableReadAccess` AST expression. */
   class VariableReadAccessCfgNode extends ExprCfgNode {
     override VariableReadAccess e;

ruby/ql/lib/codeql/ruby/dataflow/Sanitizers.qll

@@ -0,0 +1,16 @@
+/** Provides commonly used dataflow sanitizers */
+
+private import codeql.ruby.AST
+private import codeql.ruby.DataFlow
+
+/**
+ * A sanitizer for flow into a string interpolation component,
+ * provided that component does not form a prefix of the string.
+ *
+ * This is useful for URLs and paths, where the fixed prefix prevents the user from controlling the target.
+ */
+class PrefixedStringInterpolation extends DataFlow::Node {
+  PrefixedStringInterpolation() {
+    exists(StringlikeLiteral str, int n | str.getComponent(n) = this.asExpr().getExpr() and n > 0)
+  }
+}

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll

@@ -18,12 +18,14 @@ private import codeql.ruby.ApiGraphs
  * https://github.com/excon/excon/blob/master/README.md
  */
 class ExconHttpRequest extends HTTP::Client::Request::Range {
-  DataFlow::Node requestUse;
+  DataFlow::CallNode requestUse;
   API::Node requestNode;
   API::Node connectionNode;
+  DataFlow::Node connectionUse;
 
   ExconHttpRequest() {
     requestUse = requestNode.getAnImmediateUse() and
+    connectionUse = connectionNode.getAnImmediateUse() and
     connectionNode =
       [
         // one-off requests
@@ -44,6 +46,17 @@ class ExconHttpRequest extends HTTP::Client::Request::Range {
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
 
+  override DataFlow::Node getURL() {
+    // For one-off requests, the URL is in the first argument of the request method call.
+    // For connection re-use, the URL is split between the first argument of the `new` call
+    // and the `path` keyword argument of the request method call.
+    result = requestUse.getArgument(0) and not result.asExpr().getExpr() instanceof Pair
+    or
+    result = requestUse.getKeywordArgument("path")
+    or
+    result = connectionUse.(DataFlow::CallNode).getArgument(0)
+  }
+
   override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
     // Check for `ssl_verify_peer: false` in the options hash.
     exists(DataFlow::Node arg, int i |

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Faraday.qll

@@ -11,12 +11,16 @@ private import codeql.ruby.ApiGraphs
  * # connection re-use
  * connection = Faraday.new("http://example.com")
  * connection.get("/").body
+ *
+ * connection = Faraday.new(url: "http://example.com")
+ * connection.get("/").body
  * ```
  */
 class FaradayHttpRequest extends HTTP::Client::Request::Range {
-  DataFlow::Node requestUse;
   API::Node requestNode;
   API::Node connectionNode;
+  DataFlow::Node connectionUse;
+  DataFlow::CallNode requestUse;
 
   FaradayHttpRequest() {
     connectionNode =
@@ -29,11 +33,18 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
     requestNode =
       connectionNode.getReturn(["get", "head", "delete", "post", "put", "patch", "trace"]) and
     requestUse = requestNode.getAnImmediateUse() and
+    connectionUse = connectionNode.getAnImmediateUse() and
     this = requestUse.asExpr().getExpr()
   }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
 
+  override DataFlow::Node getURL() {
+    result = requestUse.getArgument(0) or
+    result = connectionUse.(DataFlow::CallNode).getArgument(0) or
+    result = connectionUse.(DataFlow::CallNode).getKeywordArgument("url")
+  }
+
   override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
     // `Faraday::new` takes an options hash as its second argument, and we're
     // looking for

ruby/ql/lib/codeql/ruby/frameworks/http_clients/HttpClient.qll

@@ -12,7 +12,7 @@ private import codeql.ruby.ApiGraphs
 class HttpClientRequest extends HTTP::Client::Request::Range {
   API::Node requestNode;
   API::Node connectionNode;
-  DataFlow::Node requestUse;
+  DataFlow::CallNode requestUse;
   string method;
 
   HttpClientRequest() {
@@ -31,6 +31,8 @@ class HttpClientRequest extends HTTP::Client::Request::Range {
     this = requestUse.asExpr().getExpr()
   }
 
+  override DataFlow::Node getURL() { result = requestUse.getArgument(0) }
+
   override DataFlow::Node getResponseBody() {
     // The `get_content` and `post_content` methods return the response body as
     // a string. The other methods return a `HTTPClient::Message` object which

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Httparty.qll

@@ -11,14 +11,15 @@ private import codeql.ruby.ApiGraphs
  * # TODO: module inclusion
  * class MyClass
  *  include HTTParty
+ *  base_uri "http://example.com"
  * end
  *
  * MyClass.new("http://example.com")
  * ```
  */
 class HttpartyRequest extends HTTP::Client::Request::Range {
   API::Node requestNode;
-  DataFlow::Node requestUse;
+  DataFlow::CallNode requestUse;
 
   HttpartyRequest() {
     requestUse = requestNode.getAnImmediateUse() and
@@ -28,6 +29,8 @@ class HttpartyRequest extends HTTP::Client::Request::Range {
     this = requestUse.asExpr().getExpr()
   }
 
+  override DataFlow::Node getURL() { result = requestUse.getArgument(0) }
+
   override DataFlow::Node getResponseBody() {
     // If HTTParty can recognise the response type, it will parse and return it
     // directly from the request call. Otherwise, it will return a `HTTParty::Response`

ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll

@@ -46,7 +46,7 @@ class NetHttpRequest extends HTTP::Client::Request::Range {
    * Gets the node representing the URL of the request.
    * Currently unused, but may be useful in future, e.g. to filter out certain requests.
    */
-  DataFlow::Node getURLArgument() { result = request.getArgument(0) }
+  override DataFlow::Node getURL() { result = request.getArgument(0) }
 
   override DataFlow::Node getResponseBody() { result = responseBody }
 

ruby/ql/lib/codeql/ruby/frameworks/http_clients/OpenURI.qll

@@ -14,7 +14,7 @@ private import codeql.ruby.frameworks.StandardLibrary
  */
 class OpenUriRequest extends HTTP::Client::Request::Range {
   API::Node requestNode;
-  DataFlow::Node requestUse;
+  DataFlow::CallNode requestUse;
 
   OpenUriRequest() {
     requestNode =
@@ -24,6 +24,8 @@ class OpenUriRequest extends HTTP::Client::Request::Range {
     this = requestUse.asExpr().getExpr()
   }
 
+  override DataFlow::Node getURL() { result = requestUse.getArgument(0) }
+
   override DataFlow::Node getResponseBody() {
     result = requestNode.getAMethodCall(["read", "readlines"])
   }
@@ -48,14 +50,16 @@ class OpenUriRequest extends HTTP::Client::Request::Range {
  * ```
  */
 class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range {
-  DataFlow::Node requestUse;
+  DataFlow::CallNode requestUse;
 
   OpenUriKernelOpenRequest() {
     requestUse instanceof KernelMethodCall and
     this.getMethodName() = "open" and
     this = requestUse.asExpr().getExpr()
   }
 
+  override DataFlow::Node getURL() { result = requestUse.getArgument(0) }
+
   override DataFlow::CallNode getResponseBody() {
     result.asExpr().getExpr().(MethodCall).getMethodName() in ["read", "readlines"] and
     requestUse.(DataFlow::LocalSourceNode).flowsTo(result.getReceiver())