Merge pull request #10114 from RasmusWL/shared-http-client-request

Ruby/Python: Shared HTTP client request concept

Author: RasmusWL

config/identical-files.json

@@ -30,6 +30,7 @@
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImpl2.qll",
     "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForLibraries.qll",
+    "ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplForHttpClientLibraries.qll",
     "swift/ql/lib/codeql/swift/dataflow/internal/DataFlowImpl.qll"
   ],
   "DataFlow Java/C++/C#/Python Common": [

javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll

@@ -87,3 +87,70 @@ module Cryptography {
     predicate isWeak() { this = "ECB" }
   }
 }
+
+/** Provides classes for modeling HTTP-related APIs. */
+module Http {
+  /** Provides classes for modeling HTTP clients. */
+  module Client {
+    /**
+     * A data-flow node that makes an outgoing HTTP request.
+     *
+     * Extend this class to refine existing API models. If you want to model new APIs,
+     * extend `Http::Client::Request::Range` instead.
+     */
+    class Request extends DataFlow::Node instanceof Request::Range {
+      /**
+       * 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() }
+
+      /**
+       * 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, and `argumentOrigin` represents the origin
+       * of the argument that disabled the validation (which could be the same node as
+       * `disablingNode`).
+       */
+      predicate disablesCertificateValidation(
+        DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+      ) {
+        super.disablesCertificateValidation(disablingNode, argumentOrigin)
+      }
+    }
+
+    /** Provides a class for modeling new HTTP requests. */
+    module Request {
+      /**
+       * A data-flow node that makes an outgoing HTTP request.
+       *
+       * Extend this class to model new APIs. If you want to refine existing API models,
+       * extend `Http::Client::Request` instead.
+       */
+      abstract class Range extends DataFlow::Node {
+        /**
+         * 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();
+
+        /**
+         * 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, and `argumentOrigin` represents the origin
+         * of the argument that disabled the validation (which could be the same node as
+         * `disablingNode`).
+         */
+        abstract predicate disablesCertificateValidation(
+          DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+        );
+      }
+    }
+  }
+}

python/ql/lib/semmle/python/Concepts.qll

@@ -1046,71 +1046,9 @@ module HTTP {
     }
   }
 
-  /** Provides classes for modeling HTTP clients. */
-  module Client {
-    /**
-     * A data-flow node that makes an outgoing HTTP request.
-     *
-     * Extend this class to refine existing API models. If you want to model new APIs,
-     * extend `HTTP::Client::Request::Range` instead.
-     */
-    class Request extends DataFlow::Node instanceof Request::Range {
-      /**
-       * 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() }
-
-      /**
-       * 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, and `argumentOrigin` represents the origin
-       * of the argument that disabled the validation (which could be the same node as
-       * `disablingNode`).
-       */
-      predicate disablesCertificateValidation(
-        DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
-      ) {
-        super.disablesCertificateValidation(disablingNode, argumentOrigin)
-      }
-    }
-
-    /** Provides a class for modeling new HTTP requests. */
-    module Request {
-      /**
-       * A data-flow node that makes an outgoing HTTP request.
-       *
-       * Extend this class to model new APIs. If you want to refine existing API models,
-       * extend `HTTP::Client::Request` instead.
-       */
-      abstract class Range extends DataFlow::Node {
-        /**
-         * 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();
-
-        /**
-         * 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, and `argumentOrigin` represents the origin
-         * of the argument that disabled the validation (which could be the same node as
-         * `disablingNode`).
-         */
-        abstract predicate disablesCertificateValidation(
-          DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
-        );
-      }
-    }
-    // TODO: investigate whether we should treat responses to client requests as
-    // remote-flow-sources in general.
-  }
+  import semmle.python.internal.ConceptsShared::Http::Client as Client
+  // TODO: investigate whether we should treat responses to client requests as
+  // remote-flow-sources in general.
 }
 
 /**

python/ql/lib/semmle/python/internal/ConceptsShared.qll

@@ -87,3 +87,70 @@ module Cryptography {
     predicate isWeak() { this = "ECB" }
   }
 }
+
+/** Provides classes for modeling HTTP-related APIs. */
+module Http {
+  /** Provides classes for modeling HTTP clients. */
+  module Client {
+    /**
+     * A data-flow node that makes an outgoing HTTP request.
+     *
+     * Extend this class to refine existing API models. If you want to model new APIs,
+     * extend `Http::Client::Request::Range` instead.
+     */
+    class Request extends DataFlow::Node instanceof Request::Range {
+      /**
+       * 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() }
+
+      /**
+       * 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, and `argumentOrigin` represents the origin
+       * of the argument that disabled the validation (which could be the same node as
+       * `disablingNode`).
+       */
+      predicate disablesCertificateValidation(
+        DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+      ) {
+        super.disablesCertificateValidation(disablingNode, argumentOrigin)
+      }
+    }
+
+    /** Provides a class for modeling new HTTP requests. */
+    module Request {
+      /**
+       * A data-flow node that makes an outgoing HTTP request.
+       *
+       * Extend this class to model new APIs. If you want to refine existing API models,
+       * extend `Http::Client::Request` instead.
+       */
+      abstract class Range extends DataFlow::Node {
+        /**
+         * 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();
+
+        /**
+         * 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, and `argumentOrigin` represents the origin
+         * of the argument that disabled the validation (which could be the same node as
+         * `disablingNode`).
+         */
+        abstract predicate disablesCertificateValidation(
+          DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+        );
+      }
+    }
+  }
+}

python/ql/src/Security/CWE-295/RequestWithoutValidation.ql

@@ -13,7 +13,6 @@
 import python
 private import semmle.python.dataflow.new.DataFlow
 private import semmle.python.Concepts
-private import semmle.python.ApiGraphs
 
 from
   HTTP::Client::Request request, DataFlow::Node disablingNode, DataFlow::Node origin, string ending

ruby/ql/lib/change-notes/2022-08-18-change-http-client-request.md

@@ -0,0 +1,4 @@
+---
+category: breaking
+---
+* Changed the `HTTP::Client::Request` concept from using `MethodCall` as base class, to using `DataFlow::Node` as base class. Any class that extends `HTTP::Client::Request::Range` must be changed, but if you only use the member predicates of `HTTP::Client::Request`, no changes are required.

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 MethodCall 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 MethodCall {
+      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: override `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() }
       }
     }