Add a query for server-side request forgery

Author: hmac

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

@@ -0,0 +1,47 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * server side request forgery, as well as extension points for adding your own.
+ */
+
+private import ruby
+private import codeql.ruby.ApiGraphs
+private import codeql.ruby.CFG
+private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.RemoteFlowSources
+private import codeql.ruby.Concepts
+private import codeql.ruby.dataflow.Sanitizers
+
+module ServerSideRequestForgery {
+  /**
+   * A data flow source for server side request forgery vulnerabilities.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for server side request forgery vulnerabilities.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for server side request forgery vulnerabilities.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A sanitizer guard for "URL redirection" vulnerabilities.
+   */
+  abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
+
+  /** A source of remote user input, considered as a flow source for server side request forgery. */
+  class RemoteFlowSourceAsSource extends Source {
+    RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
+  }
+
+  /** The URL of an HTTP request, considered as a sink. */
+  class HttpRequestAsSink extends Sink {
+    HttpRequestAsSink() { exists(HTTP::Client::Request req | req.getURL() = this) }
+  }
+
+  /** String interpolation with a fixed prefix, considered as a flow sanitizer. */
+  class StringInterpolationAsSanitizer extends PrefixedStringInterpolation, Sanitizer { }
+}

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

@@ -0,0 +1,33 @@
+/**
+ * Provides a taint-tracking configuration for detecting
+ * "Server side request forgery" vulnerabilities.
+ *
+ * Note, for performance reasons: only import this file if `Configuration` is needed,
+ * otherwise `ServerSideRequestForgeryCustomizations` should be imported instead.
+ */
+
+import codeql.ruby.DataFlow::DataFlow::PathGraph
+import codeql.ruby.DataFlow
+import codeql.ruby.TaintTracking
+import ServerSideRequestForgeryCustomizations::ServerSideRequestForgery
+import codeql.ruby.dataflow.BarrierGuards
+
+/**
+ * A taint-tracking configuration for detecting
+ * "Server side request forgery" vulnerabilities.
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "ServerSideRequestForgery" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof SanitizerGuard or
+    guard instanceof StringConstCompare or
+    guard instanceof StringConstArrayInclusionCall
+  }
+}

ruby/ql/src/queries/security/cwe-918/ServerSideRequestForgery.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Server Side Request Forgery
+ * @description Making a request to a URL that is controlled by user input
+ *              can allow an attacker to forge requests to internal services.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity TODO
+ * @precision medium
+ * @id rb/server-side-request-forgery
+ * @tags security
+ *       external/cwe/cwe-918
+ */
+
+import ruby
+import codeql.ruby.Concepts
+import codeql.ruby.DataFlow
+import codeql.ruby.TaintTracking
+import codeql.ruby.dataflow.RemoteFlowSources
+import codeql.ruby.security.ServerSideRequestForgeryQuery
+
+from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Untrusted HTTP request due to $@.", source.getNode(),
+  "a user-provided value"

ruby/ql/test/query-tests/security/cwe-918/ServerSideRequestForgery.expected

@@ -0,0 +1,8 @@
+edges
+| ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" |
+nodes
+| ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | semmle.label | call to params :  |
+| ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | semmle.label | "#{...}/logins" |
+subpaths
+#select
+| ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | Untrusted HTTP request due to $@. | ServerSideRequestForgery.rb:9:32:9:37 | call to params | a user-provided value |

ruby/ql/test/query-tests/security/cwe-918/ServerSideRequestForgery.qlref

@@ -0,0 +1 @@
+queries/security/cwe-918/ServerSideRequestForgery.ql

ruby/ql/test/query-tests/security/cwe-918/ServerSideRequestForgery.rb

@@ -0,0 +1,21 @@
+require "Excon"
+require "json"
+
+class PostsController < ActionController::Base
+    def create
+        user = params[:user_id]
+
+        # BAD - user can control the entire URL of the request
+        users_service_domain = params[:users_service_domain]
+        response = Excon.post("#{users_service_domain}/logins", body: {user_id: user}).body
+        token = JSON.parse(response)["token"]
+
+        # GOOD - user can only control the suffix of the URL
+        users_service_path = params[:users_service_path]
+        response = Excon.post("users-service/#{users_service_path}", body: {user_id: user}).body
+        token = JSON.parse(response)["token"]
+
+        @post = Post.create(params[:post].merge(user_token: token))
+        render @post
+    end
+end