Ruby: Base HTTP::Client::Request on shared concept

Fixing up deprecation errors in next commit

Author: RasmusWL

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

@@ -474,13 +474,15 @@ module HTTP {
 
   /** Provides classes for modeling HTTP clients. */
   module Client {
+    import codeql.ruby.internal.ConceptsShared::Http::Client as SC
+
     /**
      * A method call that makes an outgoing HTTP request.
      *
      * Extend this class to refine existing API models. If you want to model new APIs,
      * extend `Request::Range` instead.
      */
-    class Request extends DataFlow::Node instanceof Request::Range {
+    class Request extends SC::Request instanceof Request::Range {
       /** Gets a node which returns the body of the response */
       DataFlow::Node getResponseBody() { result = super.getResponseBody() }
 
@@ -490,24 +492,19 @@ module HTTP {
        * 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.
        */
-      deprecated DataFlow::Node getURL() { result = super.getURL() or result = super.getAUrlPart() }
-
-      /**
-       * Gets a data-flow 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 getAUrlPart() { result = super.getAUrlPart() }
-
-      /** Gets a string that identifies the framework used for this request. */
-      string getFramework() { result = super.getFramework() }
+      deprecated DataFlow::Node getURL() {
+        result = super.getURL() or result = Request::Range.super.getAUrlPart()
+      }
 
       /**
        * Holds if this request is made using a mode that disables SSL/TLS
        * certificate validation, where `disablingNode` represents the point at
        * which the validation was disabled.
        */
-      predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
-        super.disablesCertificateValidation(disablingNode)
+      deprecated predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+        Request::Range.super.disablesCertificateValidation(disablingNode, _)
+        or
+        Request::Range.super.disablesCertificateValidation(disablingNode)
       }
     }
 
@@ -519,7 +516,7 @@ module HTTP {
        * Extend this class to model new APIs. If you want to refine existing API models,
        * extend `Request` instead.
        */
-      abstract class Range extends DataFlow::Node {
+      abstract class Range extends SC::Request::Range {
         /** Gets a node which returns the body of the response */
         abstract DataFlow::Node getResponseBody();
 
@@ -532,20 +529,13 @@ module HTTP {
         deprecated DataFlow::Node getURL() { none() }
 
         /**
-         * Gets a data-flow 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 getAUrlPart();
-
-        /** Gets a string that identifies the framework used for this request. */
-        abstract string getFramework();
-
-        /**
+         * DEPRECATED: overwrite `disablesCertificateValidation/2` instead.
+         *
          * Holds if this request is made using a mode that disables SSL/TLS
          * certificate validation, where `disablingNode` represents the point at
          * which the validation was disabled.
          */
-        abstract predicate disablesCertificateValidation(DataFlow::Node disablingNode);
+        deprecated predicate disablesCertificateValidation(DataFlow::Node disablingNode) { none() }
       }
     }
 

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

@@ -86,6 +86,13 @@ class ExconHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "Excon" }
 }
 

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

@@ -22,13 +22,11 @@ private import codeql.ruby.DataFlow
  * connection.get("/").body
  * ```
  */
-
 class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
   API::Node connectionNode;
   DataFlow::Node connectionUse;
 
-
   FaradayHttpRequest() {
     connectionNode =
       [
@@ -41,7 +39,6 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNod
       connectionNode.getReturn(["get", "head", "delete", "post", "put", "patch", "trace"]) and
     this = requestNode.asSource() and
     connectionUse = connectionNode.asSource()
-
   }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
@@ -78,6 +75,13 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNod
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "Faraday" }
 }
 

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

@@ -58,5 +58,12 @@ class HttpClientRequest extends HTTP::Client::Request::Range, DataFlow::CallNode
           .getAValueReachableFromSource()
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "HTTPClient" }
 }

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

@@ -23,16 +23,14 @@ private import codeql.ruby.DataFlow
  * MyClass.new("http://example.com")
  * ```
  */
-class HttpartyRequest extends HTTP::Client::Request::Range,DataFlow::CallNode {
+class HttpartyRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
 
-
   HttpartyRequest() {
     this = requestNode.asSource() and
     requestNode =
       API::getTopLevelMember("HTTParty")
           .getReturn(["get", "head", "delete", "options", "post", "put", "patch"])
-
   }
 
   override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
@@ -73,6 +71,13 @@ class HttpartyRequest extends HTTP::Client::Request::Range,DataFlow::CallNode {
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "HTTParty" }
 }
 

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

@@ -83,5 +83,12 @@ class NetHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "Net::HTTP" }
 }

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

@@ -43,6 +43,13 @@ class OpenUriRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "OpenURI" }
 }
 
@@ -75,6 +82,13 @@ class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range, DataFlow::C
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "OpenURI" }
 }
 

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

@@ -17,13 +17,11 @@ private import codeql.ruby.DataFlow
  * ```
  */
 class RestClientHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-
   API::Node requestNode;
   API::Node connectionNode;
 
   RestClientHttpRequest() {
     this = requestNode.asSource() and
-
     (
       connectionNode =
         [
@@ -71,6 +69,13 @@ class RestClientHttpRequest extends HTTP::Client::Request::Range, DataFlow::Call
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "RestClient" }
 }
 

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

@@ -15,15 +15,13 @@ private import codeql.ruby.DataFlow
  * ```
  */
 class TyphoeusHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-
   API::Node requestNode;
 
   TyphoeusHttpRequest() {
     this = requestNode.asSource() and
     requestNode =
       API::getTopLevelMember("Typhoeus")
           .getReturn(["get", "head", "delete", "options", "post", "put", "patch"])
-
   }
 
   override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
@@ -51,6 +49,13 @@ class TyphoeusHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNo
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    disablesCertificateValidation(disablingNode) and
+    argumentOrigin = disablingNode
+  }
+
   override string getFramework() { result = "Typhoeus" }
 }
 

ruby/ql/test/query-tests/security/cwe-295/RequestWithoutValidation.expected

@@ -1,3 +1,4 @@
+WARNING: Predicate disablesCertificateValidation has been deprecated and may be removed in future (/home/rasmus/work/code/ql/ruby/ql/src/queries/security/cwe-295/RequestWithoutValidation.ql:19,15-44)
 | Excon.rb:6:3:6:34 | call to get | This request may run with $@. | Excon.rb:5:3:5:34 | call to []= | certificate validation disabled |
 | Excon.rb:12:3:12:34 | call to get | This request may run with $@. | Excon.rb:11:3:11:23 | call to ssl_verify_peer= | certificate validation disabled |
 | Excon.rb:18:3:18:34 | call to get | This request may run with $@. | Excon.rb:17:3:17:34 | call to []= | certificate validation disabled |