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

Ruby/Python: Shared HTTP client request concept

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. */
+  import semmle.python.internal.ConceptsShared::Http::Client as Client
-  module Client {
+  // TODO: investigate whether we should treat responses to client requests as
-    /**
+  // remote-flow-sources in general.
-     * 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.
-  }
 }
 
 /**

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() }
+      deprecated DataFlow::Node getURL() {
-
+        result = super.getURL() or result = Request::Range.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() }
 
       /**
        * 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) {
+      deprecated predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
-        super.disablesCertificateValidation(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.
+         * DEPRECATED: override `disablesCertificateValidation/2` instead.
-         * 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.
          */
-        abstract predicate disablesCertificateValidation(DataFlow::Node disablingNode);
+        deprecated predicate disablesCertificateValidation(DataFlow::Node disablingNode) { none() }
       }
     }
 

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

@@ -87,6 +87,41 @@ class CallNode extends LocalSourceNode, ExprNode {
 
   /** Gets the block of this call. */
   Node getBlock() { result.asExpr() = node.getBlock() }
+
+  /**
+   * Gets the data-flow node corresponding to the named argument of the call
+   * corresponding to this data-flow node, also including values passed with (pre Ruby
+   * 2.0) hash arguments.
+   *
+   * Such hash arguments are tracked back to their source location within functions, but
+   * no inter-procedural analysis occurs.
+   *
+   * This means all 3 variants below will be handled by this predicate:
+   *
+   * ```ruby
+   * foo(..., some_option: 42)
+   * foo(..., { some_option: 42 })
+   * options = { some_option: 42 }
+   * foo(..., options)
+   * ```
+   */
+  Node getKeywordArgumentIncludeHashArgument(string name) {
+    // to reduce number of computed tuples, I have put bindingset on both this and name,
+    // meaning we only do the local backwards tracking for known calls and known names.
+    // (not because a performance problem was seen, it just seemed right).
+    result = this.getKeywordArgument(name)
+    or
+    exists(CfgNodes::ExprNodes::PairCfgNode pair |
+      pair =
+        this.getArgument(_)
+            .getALocalSource()
+            .asExpr()
+            .(CfgNodes::ExprNodes::HashLiteralCfgNode)
+            .getAKeyValuePair() and
+      pair.getKey().getConstantValue().isStringlikeValue(name) and
+      result.asExpr() = pair.getValue()
+    )
+  }
 }
 
 /**

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

@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `Excon`.
@@ -23,14 +24,13 @@ private import codeql.ruby.DataFlow
  * TODO: pipelining, streaming responses
  * https://github.com/excon/excon/blob/master/README.md
  */
-class ExconHttpRequest extends HTTP::Client::Request::Range {
+class ExconHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-  DataFlow::CallNode requestUse;
   API::Node requestNode;
   API::Node connectionNode;
   DataFlow::Node connectionUse;
 
   ExconHttpRequest() {
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
     connectionUse = connectionNode.asSource() and
     connectionNode =
       [
@@ -46,8 +46,7 @@ class ExconHttpRequest extends HTTP::Client::Request::Range {
               // Excon#request exists but Excon.request doesn't.
               // This shouldn't be a problem - in real code the latter would raise NoMethodError anyway.
               "get", "head", "delete", "options", "post", "put", "patch", "trace", "request"
-            ]) and
+            ])
-    this = requestUse.asExpr().getExpr()
   }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
@@ -56,96 +55,76 @@ class ExconHttpRequest extends HTTP::Client::Request::Range {
     // 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
+    result = this.getArgument(0) and not result.asExpr().getExpr() instanceof Pair
     or
-    result = requestUse.getKeywordArgument("path")
+    result = this.getKeywordArgument("path")
     or
     result = connectionUse.(DataFlow::CallNode).getArgument(0)
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    // Check for `ssl_verify_peer: false` in the options hash.
+  DataFlow::Node getCertificateValidationControllingValue() {
-    exists(DataFlow::Node arg, int i |
+    exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
-      i > 0 and
+      // Check for `ssl_verify_peer: false`
-      arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
+      result = newCall.getKeywordArgumentIncludeHashArgument("ssl_verify_peer")
-    |
-      argSetsVerifyPeer(arg, false, disablingNode)
     )
+  }
+
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(ExconDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
     or
-    // Or we see a call to `Excon.defaults[:ssl_verify_peer] = false` before the
+    // We set `Excon.defaults[:ssl_verify_peer]` or `Excon.ssl_verify_peer` = false`
-    // request, and no `ssl_verify_peer: true` in the explicit options hash for
+    // before the request, and no `ssl_verify_peer: true` in the explicit options hash
-    // the request call.
+    // for the request call.
-    exists(DataFlow::CallNode disableCall |
+    exists(DataFlow::CallNode disableCall, BooleanLiteral value |
-      setsDefaultVerification(disableCall, false) and
+      // Excon.defaults[:ssl_verify_peer]
-      disableCall.asExpr().getASuccessor+() = requestUse.asExpr() and
+      disableCall = API::getTopLevelMember("Excon").getReturn("defaults").getAMethodCall("[]=") and
-      disablingNode = disableCall and
+      disableCall
-      not exists(DataFlow::Node arg, int i |
+          .getArgument(0)
-        i > 0 and
+          .getALocalSource()
-        arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
+          .asExpr()
-      |
+          .getConstantValue()
-        argSetsVerifyPeer(arg, true, _)
+          .isStringlikeValue("ssl_verify_peer") and
-      )
+      disablingNode = disableCall.getArgument(1) and
+      argumentOrigin = disablingNode.getALocalSource() and
+      value = argumentOrigin.asExpr().getExpr()
+      or
+      // Excon.ssl_verify_peer
+      disableCall = API::getTopLevelMember("Excon").getAMethodCall("ssl_verify_peer=") and
+      disablingNode = disableCall.getArgument(0) and
+      argumentOrigin = disablingNode.getALocalSource() and
+      value = argumentOrigin.asExpr().getExpr()
+    |
+      value.getValue() = false and
+      disableCall.asExpr().getASuccessor+() = this.asExpr() and
+      // no `ssl_verify_peer: true` in the request call.
+      not this.getCertificateValidationControllingValue()
+          .getALocalSource()
+          .asExpr()
+          .getExpr()
+          .(BooleanLiteral)
+          .getValue() = true
     )
   }
 
   override string getFramework() { result = "Excon" }
 }
 
-/**
+/** A configuration to track values that can disable certificate validation for Excon. */
- * Holds if `arg` represents an options hash that contains the key
+private class ExconDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
- * `:ssl_verify_peer` with `value`, where `kvNode` is the data-flow node for
+  ExconDisablesCertificateValidationConfiguration() {
- * this key-value pair.
+    this = "ExconDisablesCertificateValidationConfiguration"
- */
+  }
-predicate argSetsVerifyPeer(DataFlow::Node arg, boolean value, DataFlow::Node kvNode) {
-  // Either passed as an individual key:value argument, e.g.:
-  // Excon.get(..., ssl_verify_peer: false)
-  isSslVerifyPeerPair(arg.asExpr(), value) and
-  kvNode = arg
-  or
-  // Or as a single hash argument, e.g.:
-  // Excon.get(..., { ssl_verify_peer: false, ... })
-  exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-    p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-    isSslVerifyPeerPair(p, value) and
-    optionsNode.flowsTo(arg) and
-    kvNode.asExpr() = p
-  )
-}
 
-/**
+  override predicate isSource(DataFlow::Node source) {
- * Holds if `callNode` sets `Excon.defaults[:ssl_verify_peer]` or
+    source.asExpr().getExpr().(BooleanLiteral).isFalse()
- * `Excon.ssl_verify_peer` to `value`.
+  }
- */
-private predicate setsDefaultVerification(DataFlow::CallNode callNode, boolean value) {
-  callNode = API::getTopLevelMember("Excon").getReturn("defaults").getAMethodCall("[]=") and
-  isSslVerifyPeerLiteral(callNode.getArgument(0)) and
-  hasBooleanValue(callNode.getArgument(1), value)
-  or
-  callNode = API::getTopLevelMember("Excon").getAMethodCall("ssl_verify_peer=") and
-  hasBooleanValue(callNode.getArgument(0), value)
-}
 
-private predicate isSslVerifyPeerLiteral(DataFlow::Node node) {
+  override predicate isSink(DataFlow::Node sink) {
-  exists(DataFlow::LocalSourceNode literal |
+    sink = any(ExconHttpRequest req).getCertificateValidationControllingValue()
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue("ssl_verify_peer") and
+  }
-    literal.flowsTo(node)
-  )
-}
-
-/** Holds if `node` can contain `value`. */
-private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
-  exists(DataFlow::LocalSourceNode literal |
-    literal.asExpr().getExpr().(BooleanLiteral).getValue() = value and
-    literal.flowsTo(node)
-  )
-}
-
-/** Holds if `p` is the pair `ssl_verify_peer: <value>`. */
-private predicate isSslVerifyPeerPair(CfgNodes::ExprNodes::PairCfgNode p, boolean value) {
-  exists(DataFlow::Node key, DataFlow::Node valueNode |
-    key.asExpr() = p.getKey() and
-    valueNode.asExpr() = p.getValue() and
-    isSslVerifyPeerLiteral(key) and
-    hasBooleanValue(valueNode, value)
-  )
 }

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

@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `Faraday`.
@@ -22,11 +23,10 @@ private import codeql.ruby.DataFlow
  * connection.get("/").body
  * ```
  */
-class FaradayHttpRequest extends HTTP::Client::Request::Range {
+class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
   API::Node connectionNode;
   DataFlow::Node connectionUse;
-  DataFlow::CallNode requestUse;
 
   FaradayHttpRequest() {
     connectionNode =
@@ -38,125 +38,67 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
       ] and
     requestNode =
       connectionNode.getReturn(["get", "head", "delete", "post", "put", "patch", "trace"]) and
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
-    connectionUse = connectionNode.asSource() and
+    connectionUse = connectionNode.asSource()
-    this = requestUse.asExpr().getExpr()
   }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
 
   override DataFlow::Node getAUrlPart() {
-    result = requestUse.getArgument(0) or
+    result = this.getArgument(0) or
     result = connectionUse.(DataFlow::CallNode).getArgument(0) or
     result = connectionUse.(DataFlow::CallNode).getKeywordArgument("url")
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any, with argument name `name`. */
+  DataFlow::Node getCertificateValidationControllingValue(string argName) {
     // `Faraday::new` takes an options hash as its second argument, and we're
     // looking for
     // `{ ssl: { verify: false } }`
     // or
     // `{ ssl: { verify_mode: OpenSSL::SSL::VERIFY_NONE } }`
-    exists(DataFlow::Node arg, int i |
+    argName in ["verify", "verify_mode"] and
-      i > 0 and
+    exists(DataFlow::Node sslValue, DataFlow::CallNode newCall |
-      arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
+      newCall = connectionNode.getAValueReachableFromSource() and
+      sslValue = newCall.getKeywordArgumentIncludeHashArgument("ssl")
     |
-      // Either passed as an individual key:value argument, e.g.:
+      exists(CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key |
-      // Faraday.new(..., ssl: {...})
+        p = sslValue.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-      isSslOptionsPairDisablingValidation(arg.asExpr()) and
+        key.asExpr() = p.getKey() and
-      disablingNode = arg
+        key.getALocalSource().asExpr().getConstantValue().isStringlikeValue(argName) and
-      or
+        result.asExpr() = p.getValue()
-      // Or as a single hash argument, e.g.:
-      // Faraday.new(..., { ssl: {...} })
-      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-        isSslOptionsPairDisablingValidation(p) and
-        optionsNode.flowsTo(arg) and
-        disablingNode.asExpr() = p
       )
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(FaradayDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue(_)
+  }
+
   override string getFramework() { result = "Faraday" }
 }
 
-/**
+/** A configuration to track values that can disable certificate validation for Faraday. */
- * Holds if the pair `p` contains the key `:ssl` for which the value is a hash
+private class FaradayDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
- * containing either `verify: false` or
+  FaradayDisablesCertificateValidationConfiguration() {
- * `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
+    this = "FaradayDisablesCertificateValidationConfiguration"
- */
+  }
-private predicate isSslOptionsPairDisablingValidation(CfgNodes::ExprNodes::PairCfgNode p) {
-  exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr() = p.getKey() and
-    value.asExpr() = p.getValue() and
-    isSymbolLiteral(key, "ssl") and
-    (isHashWithVerifyFalse(value) or isHashWithVerifyModeNone(value))
-  )
-}
 
-/** Holds if `node` represents the symbol literal with the given `valueText`. */
+  override predicate isSource(
-private predicate isSymbolLiteral(DataFlow::Node node, string valueText) {
+    DataFlow::Node source, DataFlowImplForHttpClientLibraries::FlowState state
-  exists(DataFlow::LocalSourceNode literal |
+  ) {
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue(valueText) and
+    source.asExpr().getExpr().(BooleanLiteral).isFalse() and
-    literal.flowsTo(node)
+    state = "verify"
-  )
+    or
-}
+    source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource() and
+    state = "verify_mode"
+  }
 
-/**
+  override predicate isSink(DataFlow::Node sink, DataFlowImplForHttpClientLibraries::FlowState state) {
- * Holds if `node` represents a hash containing the key-value pair
+    sink = any(FaradayHttpRequest req).getCertificateValidationControllingValue(state)
- * `verify: false`.
+  }
- */
-private predicate isHashWithVerifyFalse(DataFlow::Node node) {
-  exists(DataFlow::LocalSourceNode hash |
-    isVerifyFalsePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
-    hash.flowsTo(node)
-  )
-}
-
-/**
- * Holds if `node` represents a hash containing the key-value pair
- * `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
- */
-private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
-  exists(DataFlow::LocalSourceNode hash |
-    isVerifyModeNonePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
-    hash.flowsTo(node)
-  )
-}
-
-/**
- * Holds if the pair `p` has the key `:verify_mode` and the value
- * `OpenSSL::SSL::VERIFY_NONE`.
- */
-private predicate isVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
-  exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr() = p.getKey() and
-    value.asExpr() = p.getValue() and
-    isSymbolLiteral(key, "verify_mode") and
-    value =
-      API::getTopLevelMember("OpenSSL")
-          .getMember("SSL")
-          .getMember("VERIFY_NONE")
-          .getAValueReachableFromSource()
-  )
-}
-
-/**
- * Holds if the pair `p` has the key `:verify` and the value `false`.
- */
-private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
-  exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr() = p.getKey() and
-    value.asExpr() = p.getValue() and
-    isSymbolLiteral(key, "verify") and
-    isFalse(value)
-  )
-}
-
-/** Holds if `node` can contain the Boolean value `false`. */
-private predicate isFalse(DataFlow::Node node) {
-  exists(DataFlow::LocalSourceNode literal |
-    literal.asExpr().getExpr().(BooleanLiteral).isFalse() and
-    literal.flowsTo(node)
-  )
 }

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

@@ -6,6 +6,7 @@ private import ruby
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `HTTPClient`.
@@ -14,10 +15,9 @@ private import codeql.ruby.DataFlow
  * HTTPClient.get_content("http://example.com")
  * ```
  */
-class HttpClientRequest extends HTTP::Client::Request::Range {
+class HttpClientRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
   API::Node connectionNode;
-  DataFlow::CallNode requestUse;
   string method;
 
   HttpClientRequest() {
@@ -29,36 +29,60 @@ class HttpClientRequest extends HTTP::Client::Request::Range {
         API::getTopLevelMember("HTTPClient").getInstance()
       ] and
     requestNode = connectionNode.getReturn(method) and
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
     method in [
         "get", "head", "delete", "options", "post", "put", "trace", "get_content", "post_content"
-      ] and
+      ]
-    this = requestUse.asExpr().getExpr()
   }
 
-  override DataFlow::Node getAUrlPart() { result = requestUse.getArgument(0) }
+  override DataFlow::Node getAUrlPart() { result = this.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
     // has various methods that return the response body.
-    method in ["get_content", "post_content"] and result = requestUse
+    method in ["get_content", "post_content"] and result = this
     or
     not method in ["get_content", "put_content"] and
     result = requestNode.getAMethodCall(["body", "http_body", "content", "dump"])
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
+  DataFlow::Node getCertificateValidationControllingValue() {
     // Look for calls to set
     // `c.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_NONE`
     // on an HTTPClient connection object `c`.
-    disablingNode = connectionNode.getReturn("ssl_config").getReturn("verify_mode=").asSource() and
+    result =
-    disablingNode.(DataFlow::CallNode).getArgument(0) =
+      connectionNode
-      API::getTopLevelMember("OpenSSL")
+          .getReturn("ssl_config")
-          .getMember("SSL")
+          .getReturn("verify_mode=")
-          .getMember("VERIFY_NONE")
+          .asSource()
-          .getAValueReachableFromSource()
+          .(DataFlow::CallNode)
+          .getArgument(0)
+  }
+
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(HttpClientDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
   }
 
   override string getFramework() { result = "HTTPClient" }
 }
+
+/** A configuration to track values that can disable certificate validation for HttpClient. */
+private class HttpClientDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
+  HttpClientDisablesCertificateValidationConfiguration() {
+    this = "HttpClientDisablesCertificateValidationConfiguration"
+  }
+
+  override predicate isSource(DataFlow::Node source) {
+    source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource()
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(HttpClientRequest req).getCertificateValidationControllingValue()
+  }
+}

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

@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `HTTParty`.
@@ -23,19 +24,17 @@ private import codeql.ruby.DataFlow
  * MyClass.new("http://example.com")
  * ```
  */
-class HttpartyRequest extends HTTP::Client::Request::Range {
+class HttpartyRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
-  DataFlow::CallNode requestUse;
 
   HttpartyRequest() {
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
     requestNode =
       API::getTopLevelMember("HTTParty")
-          .getReturn(["get", "head", "delete", "options", "post", "put", "patch"]) and
+          .getReturn(["get", "head", "delete", "options", "post", "put", "patch"])
-    this = requestUse.asExpr().getExpr()
   }
 
-  override DataFlow::Node getAUrlPart() { result = requestUse.getArgument(0) }
+  override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
 
   override DataFlow::Node getResponseBody() {
     // If HTTParty can recognise the response type, it will parse and return it
@@ -46,60 +45,36 @@ class HttpartyRequest extends HTTP::Client::Request::Range {
     or
     // Otherwise, treat the response as the response body.
     not exists(requestNode.getAMethodCall("body")) and
-    result = requestUse
+    result = this
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    // The various request methods take an options hash as their second
+  DataFlow::Node getCertificateValidationControllingValue() {
-    // argument, and we're looking for `{ verify: false }` or
+    result = this.getKeywordArgumentIncludeHashArgument(["verify", "verify_peer"])
-    // `{ verify_peer: false }`.
+  }
-    exists(DataFlow::Node arg, int i |
+
-      i > 0 and
+  override predicate disablesCertificateValidation(
-      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getArgument(i)
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
-    |
+  ) {
-      // Either passed as an individual key:value argument, e.g.:
+    any(HttpartyDisablesCertificateValidationConfiguration config)
-      // HTTParty.get(..., verify: false)
+        .hasFlow(argumentOrigin, disablingNode) and
-      isVerifyFalsePair(arg.asExpr()) and
+    disablingNode = this.getCertificateValidationControllingValue()
-      disablingNode = arg
-      or
-      // Or as a single hash argument, e.g.:
-      // HTTParty.get(..., { verify: false, ... })
-      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-        isVerifyFalsePair(p) and
-        optionsNode.flowsTo(arg) and
-        disablingNode.asExpr() = p
-      )
-    )
   }
 
   override string getFramework() { result = "HTTParty" }
 }
 
-/** Holds if `node` represents the symbol literal `verify` or `verify_peer`. */
+/** A configuration to track values that can disable certificate validation for Httparty. */
-private predicate isVerifyLiteral(DataFlow::Node node) {
+private class HttpartyDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
-  exists(DataFlow::LocalSourceNode literal |
+  HttpartyDisablesCertificateValidationConfiguration() {
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue(["verify", "verify_peer"]) and
+    this = "HttpartyDisablesCertificateValidationConfiguration"
-    literal.flowsTo(node)
+  }
-  )
-}
 
-/** Holds if `node` can contain the Boolean value `false`. */
+  override predicate isSource(DataFlow::Node source) {
-private predicate isFalse(DataFlow::Node node) {
+    source.asExpr().getExpr().(BooleanLiteral).isFalse()
-  exists(DataFlow::LocalSourceNode literal |
+  }
-    literal.asExpr().getExpr().(BooleanLiteral).isFalse() and
-    literal.flowsTo(node)
-  )
-}
 
-/**
+  override predicate isSink(DataFlow::Node sink) {
- * Holds if `p` is the pair `verify: false` or `verify_peer: false`.
+    sink = any(HttpartyRequest req).getCertificateValidationControllingValue()
- */
+  }
-private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
-  exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr() = p.getKey() and
-    value.asExpr() = p.getValue() and
-    isVerifyLiteral(key) and
-    isFalse(value)
-  )
 }

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

@@ -8,6 +8,7 @@ private import codeql.ruby.dataflow.RemoteFlowSources
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.dataflow.internal.DataFlowPublic
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A `Net::HTTP` call which initiates an HTTP request.
@@ -18,7 +19,7 @@ private import codeql.ruby.DataFlow
  * response = req.get("/")
  * ```
  */
-class NetHttpRequest extends HTTP::Client::Request::Range {
+class NetHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   private DataFlow::CallNode request;
   private DataFlow::Node responseBody;
   private API::Node requestNode;
@@ -26,7 +27,7 @@ class NetHttpRequest extends HTTP::Client::Request::Range {
   NetHttpRequest() {
     exists(string method |
       request = requestNode.asSource() and
-      this = request.asExpr().getExpr()
+      this = request
     |
       // Net::HTTP.get(...)
       method = "get" and
@@ -65,23 +66,42 @@ class NetHttpRequest extends HTTP::Client::Request::Range {
 
   override DataFlow::Node getResponseBody() { result = responseBody }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
+  DataFlow::Node getCertificateValidationControllingValue() {
     // A Net::HTTP request bypasses certificate validation if we see a setter
     // call like this:
     //   foo.verify_mode = OpenSSL::SSL::VERIFY_NONE
     // and then the receiver of that call flows to the receiver in the request:
     //   foo.request(...)
     exists(DataFlow::CallNode setter |
-      disablingNode =
-        API::getTopLevelMember("OpenSSL")
-            .getMember("SSL")
-            .getMember("VERIFY_NONE")
-            .getAValueReachableFromSource() and
       setter.asExpr().getExpr().(SetterMethodCall).getMethodName() = "verify_mode=" and
-      disablingNode = setter.getArgument(0) and
+      result = setter.getArgument(0) and
       localFlow(setter.getReceiver(), request.getReceiver())
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(NetHttpDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
+  }
+
   override string getFramework() { result = "Net::HTTP" }
 }
+
+/** A configuration to track values that can disable certificate validation for NetHttp. */
+private class NetHttpDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
+  NetHttpDisablesCertificateValidationConfiguration() {
+    this = "NetHttpDisablesCertificateValidationConfiguration"
+  }
+
+  override predicate isSource(DataFlow::Node source) {
+    source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource()
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(NetHttpRequest req).getCertificateValidationControllingValue()
+  }
+}

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

@@ -8,6 +8,7 @@ private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
 private import codeql.ruby.frameworks.Core
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `OpenURI` via `URI.open` or
@@ -18,9 +19,8 @@ private import codeql.ruby.frameworks.Core
  * URI.parse("http://example.com").open.read
  * ```
  */
-class OpenUriRequest extends HTTP::Client::Request::Range {
+class OpenUriRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
   API::Node requestNode;
-  DataFlow::CallNode requestUse;
 
   OpenUriRequest() {
     requestNode =
@@ -28,21 +28,26 @@ class OpenUriRequest extends HTTP::Client::Request::Range {
         [API::getTopLevelMember("URI"), API::getTopLevelMember("URI").getReturn("parse")]
             .getReturn("open"), API::getTopLevelMember("OpenURI").getReturn("open_uri")
       ] and
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource()
-    this = requestUse.asExpr().getExpr()
   }
 
-  override DataFlow::Node getAUrlPart() { result = requestUse.getArgument(0) }
+  override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
 
   override DataFlow::Node getResponseBody() {
     result = requestNode.getAMethodCall(["read", "readlines"])
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    exists(DataFlow::Node arg |
+  DataFlow::Node getCertificateValidationControllingValue() {
-      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getAnArgument() and
+    result = this.getKeywordArgumentIncludeHashArgument("ssl_verify_mode")
-      argumentDisablesValidation(arg, disablingNode)
+  }
-    )
+
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(OpenUriDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
   }
 
   override string getFramework() { result = "OpenURI" }
@@ -56,72 +61,60 @@ class OpenUriRequest extends HTTP::Client::Request::Range {
  * Kernel.open("http://example.com").read
  * ```
  */
-class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range {
+class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-  DataFlow::CallNode requestUse;
-
   OpenUriKernelOpenRequest() {
-    requestUse instanceof KernelMethodCall and
+    this instanceof KernelMethodCall and
-    this.getMethodName() = "open" and
+    this.getMethodName() = "open"
-    this = requestUse.asExpr().getExpr()
   }
 
-  override DataFlow::Node getAUrlPart() { result = requestUse.getArgument(0) }
+  override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
 
   override DataFlow::CallNode getResponseBody() {
     result.asExpr().getExpr().(MethodCall).getMethodName() in ["read", "readlines"] and
-    requestUse.(DataFlow::LocalSourceNode).flowsTo(result.getReceiver())
+    this.(DataFlow::LocalSourceNode).flowsTo(result.getReceiver())
   }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    exists(DataFlow::Node arg, int i |
+  DataFlow::Node getCertificateValidationControllingValue() {
-      i > 0 and
+    result = this.getKeywordArgument("ssl_verify_mode")
-      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getArgument(i) and
+    or
-      argumentDisablesValidation(arg, disablingNode)
+    // using a hashliteral
+    exists(
+      DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key
+    |
+      // can't flow to argument 0, since that's the URL
+      optionsNode.flowsTo(this.getArgument(any(int i | i > 0))) and
+      p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
+      key.asExpr() = p.getKey() and
+      key.getALocalSource().asExpr().getConstantValue().isStringlikeValue("ssl_verify_mode") and
+      result.asExpr() = p.getValue()
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(OpenUriDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
+  }
+
   override string getFramework() { result = "OpenURI" }
 }
 
-/**
+/** A configuration to track values that can disable certificate validation for OpenURI. */
- * Holds if the argument `arg` is an options hash that disables certificate
+private class OpenUriDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
- * validation, and `disablingNode` is the specific node representing the
+  OpenUriDisablesCertificateValidationConfiguration() {
- * `ssl_verify_mode: OpenSSL::SSL_VERIFY_NONE` pair.
+    this = "OpenUriDisablesCertificateValidationConfiguration"
- */
+  }
-private predicate argumentDisablesValidation(DataFlow::Node arg, DataFlow::Node disablingNode) {
-  // Either passed as an individual key:value argument, e.g.:
-  // URI.open(..., ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE)
-  isSslVerifyModeNonePair(arg.asExpr()) and
-  disablingNode = arg
-  or
-  // Or as a single hash argument, e.g.:
-  // URI.open(..., { ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE, ... })
-  exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-    p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-    isSslVerifyModeNonePair(p) and
-    optionsNode.flowsTo(arg) and
-    disablingNode.asExpr() = p
-  )
-}
 
-/** Holds if `p` is the pair `ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE`. */
+  override predicate isSource(DataFlow::Node source) {
-private predicate isSslVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
+    source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource()
-  exists(DataFlow::Node key, DataFlow::Node value |
+  }
-    key.asExpr() = p.getKey() and
-    value.asExpr() = p.getValue() and
-    isSslVerifyModeLiteral(key) and
-    value =
-      API::getTopLevelMember("OpenSSL")
-          .getMember("SSL")
-          .getMember("VERIFY_NONE")
-          .getAValueReachableFromSource()
-  )
-}
 
-/** Holds if `node` can represent the symbol literal `:ssl_verify_mode`. */
+  override predicate isSink(DataFlow::Node sink) {
-private predicate isSslVerifyModeLiteral(DataFlow::Node node) {
+    sink = any(OpenUriRequest req).getCertificateValidationControllingValue()
-  exists(DataFlow::LocalSourceNode literal |
+    or
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue("ssl_verify_mode") and
+    sink = any(OpenUriKernelOpenRequest req).getCertificateValidationControllingValue()
-    literal.flowsTo(node)
+  }
-  )
 }

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

@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `RestClient`.
@@ -16,14 +17,12 @@ private import codeql.ruby.DataFlow
  * RestClient::Request.execute(url: "http://example.com").body
  * ```
  */
-class RestClientHttpRequest extends HTTP::Client::Request::Range {
+class RestClientHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-  DataFlow::CallNode requestUse;
   API::Node requestNode;
   API::Node connectionNode;
 
   RestClientHttpRequest() {
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
-    this = requestUse.asExpr().getExpr() and
     (
       connectionNode =
         [
@@ -39,59 +38,44 @@ class RestClientHttpRequest extends HTTP::Client::Request::Range {
   }
 
   override DataFlow::Node getAUrlPart() {
-    result = requestUse.getKeywordArgument("url")
+    result = this.getKeywordArgument("url")
     or
-    result = requestUse.getArgument(0) and
+    result = this.getArgument(0) and
     // this rules out the alternative above
     not result.asExpr().getExpr() instanceof Pair
   }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    // `RestClient::Resource::new` takes an options hash argument, and we're
+  DataFlow::Node getCertificateValidationControllingValue() {
-    // looking for `{ verify_ssl: OpenSSL::SSL::VERIFY_NONE }`.
+    exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
-    exists(DataFlow::Node arg, int i |
+      result = newCall.getKeywordArgumentIncludeHashArgument("verify_ssl")
-      i > 0 and
-      arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
-    |
-      // Either passed as an individual key:value argument, e.g.:
-      // RestClient::Resource.new(..., verify_ssl: OpenSSL::SSL::VERIFY_NONE)
-      isVerifySslNonePair(arg.asExpr()) and
-      disablingNode = arg
-      or
-      // Or as a single hash argument, e.g.:
-      // RestClient::Resource.new(..., { verify_ssl: OpenSSL::SSL::VERIFY_NONE })
-      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-        isVerifySslNonePair(p) and
-        optionsNode.flowsTo(arg) and
-        disablingNode.asExpr() = p
-      )
     )
   }
 
+  override predicate disablesCertificateValidation(
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
+  ) {
+    any(RestClientDisablesCertificateValidationConfiguration config)
+        .hasFlow(argumentOrigin, disablingNode) and
+    disablingNode = this.getCertificateValidationControllingValue()
+  }
+
   override string getFramework() { result = "RestClient" }
 }
 
-/** Holds if `p` is the pair `verify_ssl: OpenSSL::SSL::VERIFY_NONE`. */
+/** A configuration to track values that can disable certificate validation for RestClient. */
-private predicate isVerifySslNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
+private class RestClientDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
-  exists(DataFlow::Node key, DataFlow::Node value |
+  RestClientDisablesCertificateValidationConfiguration() {
-    key.asExpr() = p.getKey() and
+    this = "RestClientDisablesCertificateValidationConfiguration"
-    value.asExpr() = p.getValue() and
+  }
-    isSslVerifyModeLiteral(key) and
-    value =
-      API::getTopLevelMember("OpenSSL")
-          .getMember("SSL")
-          .getMember("VERIFY_NONE")
-          .getAValueReachableFromSource()
-  )
-}
 
-/** Holds if `node` can represent the symbol literal `:verify_ssl`. */
+  override predicate isSource(DataFlow::Node source) {
-private predicate isSslVerifyModeLiteral(DataFlow::Node node) {
+    source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource()
-  exists(DataFlow::LocalSourceNode literal |
+  }
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue("verify_ssl") and
+
-    literal.flowsTo(node)
+  override predicate isSink(DataFlow::Node sink) {
-  )
+    sink = any(RestClientHttpRequest req).getCertificateValidationControllingValue()
+  }
 }

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

@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.DataFlow
+private import codeql.ruby.dataflow.internal.DataFlowImplForHttpClientLibraries as DataFlowImplForHttpClientLibraries
 
 /**
  * A call that makes an HTTP request using `Typhoeus`.
@@ -14,68 +15,47 @@ private import codeql.ruby.DataFlow
  * Typhoeus.get("http://example.com").body
  * ```
  */
-class TyphoeusHttpRequest extends HTTP::Client::Request::Range {
+class TyphoeusHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
-  DataFlow::CallNode requestUse;
   API::Node requestNode;
 
   TyphoeusHttpRequest() {
-    requestUse = requestNode.asSource() and
+    this = requestNode.asSource() and
     requestNode =
       API::getTopLevelMember("Typhoeus")
-          .getReturn(["get", "head", "delete", "options", "post", "put", "patch"]) and
+          .getReturn(["get", "head", "delete", "options", "post", "put", "patch"])
-    this = requestUse.asExpr().getExpr()
   }
 
-  override DataFlow::Node getAUrlPart() { result = requestUse.getArgument(0) }
+  override DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
 
   override DataFlow::Node getResponseBody() { result = requestNode.getAMethodCall("body") }
 
-  override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
+  /** Gets the value that controls certificate validation, if any. */
-    // Check for `ssl_verifypeer: false` in the options hash.
+  DataFlow::Node getCertificateValidationControllingValue() {
-    exists(DataFlow::Node arg, int i |
+    result = this.getKeywordArgumentIncludeHashArgument("ssl_verifypeer")
-      i > 0 and arg.asExpr().getExpr() = requestUse.asExpr().getExpr().(MethodCall).getArgument(i)
+  }
-    |
+
-      // Either passed as an individual key:value argument, e.g.:
+  override predicate disablesCertificateValidation(
-      // Typhoeus.get(..., ssl_verifypeer: false)
+    DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
-      isSslVerifyPeerFalsePair(arg.asExpr()) and
+  ) {
-      disablingNode = arg
+    any(TyphoeusDisablesCertificateValidationConfiguration config)
-      or
+        .hasFlow(argumentOrigin, disablingNode) and
-      // Or as a single hash argument, e.g.:
+    disablingNode = this.getCertificateValidationControllingValue()
-      // Typhoeus.get(..., { ssl_verifypeer: false, ... })
-      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
-        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
-        isSslVerifyPeerFalsePair(p) and
-        optionsNode.flowsTo(arg) and
-        disablingNode.asExpr() = p
-      )
-    )
   }
 
   override string getFramework() { result = "Typhoeus" }
 }
 
-/** Holds if `p` is the pair `ssl_verifypeer: false`. */
+/** A configuration to track values that can disable certificate validation for Typhoeus. */
-private predicate isSslVerifyPeerFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
+private class TyphoeusDisablesCertificateValidationConfiguration extends DataFlowImplForHttpClientLibraries::Configuration {
-  exists(DataFlow::Node key, DataFlow::Node value |
+  TyphoeusDisablesCertificateValidationConfiguration() {
-    key.asExpr() = p.getKey() and
+    this = "TyphoeusDisablesCertificateValidationConfiguration"
-    value.asExpr() = p.getValue() and
+  }
-    isSslVerifyPeerLiteral(key) and
-    isFalse(value)
-  )
-}
 
-/** Holds if `node` represents the symbol literal `verify` or `verify_peer`. */
+  override predicate isSource(DataFlow::Node source) {
-private predicate isSslVerifyPeerLiteral(DataFlow::Node node) {
+    source.asExpr().getExpr().(BooleanLiteral).isFalse()
-  exists(DataFlow::LocalSourceNode literal |
+  }
-    literal.asExpr().getExpr().getConstantValue().isStringlikeValue("ssl_verifypeer") and
-    literal.flowsTo(node)
-  )
-}
 
-/** Holds if `node` can contain the Boolean value `false`. */
+  override predicate isSink(DataFlow::Node sink) {
-private predicate isFalse(DataFlow::Node node) {
+    sink = any(TyphoeusHttpRequest req).getCertificateValidationControllingValue()
-  exists(DataFlow::LocalSourceNode literal |
+  }
-    literal.asExpr().getExpr().(BooleanLiteral).isFalse() and
-    literal.flowsTo(node)
-  )
 }

ruby/ql/lib/codeql/ruby/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
+        );
+      }
+    }
+  }
+}

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

@@ -143,7 +143,7 @@ module InsecureDownload {
       hasUnsafeExtension(req.getAUrlPart().asExpr().getConstantValue().getString())
     }
 
-    override DataFlow::Node getDownloadCall() { result.asExpr().getExpr() = req }
+    override DataFlow::Node getDownloadCall() { result = req }
 
     override DataFlow::FlowState getALabel() {
       result instanceof Label::SensitiveInsecure
@@ -193,6 +193,6 @@ module InsecureDownload {
 
     override DataFlow::FlowState getALabel() { result instanceof Label::Insecure }
 
-    override DataFlow::Node getDownloadCall() { result.asExpr().getExpr() = request }
+    override DataFlow::Node getDownloadCall() { result = request }
   }
 }

ruby/ql/src/queries/security/cwe-295/RequestWithoutValidation.ql

@@ -15,6 +15,20 @@ import ruby
 import codeql.ruby.Concepts
 import codeql.ruby.DataFlow
 
-from HTTP::Client::Request request, DataFlow::Node disablingNode
+from
-where request.disablesCertificateValidation(disablingNode)
+  HTTP::Client::Request request, DataFlow::Node disablingNode, DataFlow::Node origin, string ending
-select request, "This request may run with $@.", disablingNode, "certificate validation disabled"
+where
+  request.disablesCertificateValidation(disablingNode, origin) and
+  // Showing the origin is only useful when it's a different node than the one disabling
+  // certificate validation, for example in `requests.get(..., verify=arg)`, `arg` would
+  // be the `disablingNode`, and the `origin` would be the place were `arg` got its
+  // value from.
+  //
+  // NOTE: We compare the locations instead of DataFlow::Nodes directly, since for
+  // snippet `Excon.defaults[:ssl_verify_peer] = false`, `disablingNode = argumentNode`
+  // does NOT hold.
+  if disablingNode.getLocation() = origin.getLocation()
+  then ending = "."
+  else ending = " by the value from $@."
+select request, "This request may run without certificate validation because it is $@" + ending,
+  disablingNode, "disabled here", origin, "here"

ruby/ql/test/query-tests/security/cwe-295/Faraday.rb

@@ -26,3 +26,22 @@ response = connection.get("/")
 # GOOD
 connection = Faraday.new("http://example.com", ssl: { verify_mode: OpenSSL::SSL::VERIFY_PEER })
 response = connection.get("/")
+
+# -- example of passing verify as argument --
+
+def verify_as_arg(host, path, arg)
+    # BAD, due to the call below
+    connection = Faraday.new(host, ssl: { verify: arg })
+    response = connection.get(path)
+end
+
+verify_as_arg("http://example.com", "/", false)
+
+
+def verify_mode_as_arg(host, path, arg)
+    # BAD, due to the call below
+    connection = Faraday.new(host, ssl: { verify_mode: arg })
+    response = connection.get(path)
+end
+
+verify_mode_as_arg("http://example.com", "/", OpenSSL::SSL::VERIFY_NONE)

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

@@ -1,25 +1,28 @@
-| 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:6:3:6:34 | call to get | This request may run without certificate validation because it is $@. | Excon.rb:5:38:5:42 | ... = ... | disabled here | Excon.rb:5:38:5:42 | false | here |
-| 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:12:3:12:34 | call to get | This request may run without certificate validation because it is $@. | Excon.rb:11:27:11:31 | ... = ... | disabled here | Excon.rb:11:27:11:31 | false | here |
-| 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 |
+| Excon.rb:18:3:18:34 | call to get | This request may run without certificate validation because it is $@ by the value from $@. | Excon.rb:17:38:17:60 | ... = ... | disabled here | Excon.rb:17:55:17:59 | false | here |
-| Excon.rb:24:3:24:10 | call to get | This request may run with $@. | Excon.rb:23:55:23:76 | Pair | certificate validation disabled |
+| Excon.rb:24:3:24:10 | call to get | This request may run without certificate validation because it is $@. | Excon.rb:23:72:23:76 | false | disabled here | Excon.rb:23:72:23:76 | false | here |
-| Excon.rb:30:3:30:62 | call to get | This request may run with $@. | Excon.rb:30:36:30:57 | Pair | certificate validation disabled |
+| Excon.rb:30:3:30:62 | call to get | This request may run without certificate validation because it is $@. | Excon.rb:30:53:30:57 | false | disabled here | Excon.rb:30:53:30:57 | false | here |
-| Faraday.rb:5:12:5:30 | call to get | This request may run with $@. | Faraday.rb:4:48:4:69 | Pair | certificate validation disabled |
+| Faraday.rb:5:12:5:30 | call to get | This request may run without certificate validation because it is $@. | Faraday.rb:4:63:4:67 | false | disabled here | Faraday.rb:4:63:4:67 | false | here |
-| Faraday.rb:9:12:9:30 | call to get | This request may run with $@. | Faraday.rb:8:48:8:94 | Pair | certificate validation disabled |
+| Faraday.rb:9:12:9:30 | call to get | This request may run without certificate validation because it is $@. | Faraday.rb:8:68:8:92 | VERIFY_NONE | disabled here | Faraday.rb:8:68:8:92 | VERIFY_NONE | here |
-| HttpClient.rb:6:1:6:33 | call to get | This request may run with $@. | HttpClient.rb:5:1:5:29 | call to verify_mode= | certificate validation disabled |
+| Faraday.rb:35:16:35:35 | call to get | This request may run without certificate validation because it is $@ by the value from $@. | Faraday.rb:34:51:34:53 | arg | disabled here | Faraday.rb:38:42:38:46 | false | here |
-| Httparty.rb:4:1:4:50 | call to get | This request may run with $@. | Httparty.rb:4:37:4:49 | Pair | certificate validation disabled |
+| Faraday.rb:44:16:44:35 | call to get | This request may run without certificate validation because it is $@ by the value from $@. | Faraday.rb:43:56:43:58 | arg | disabled here | Faraday.rb:47:47:47:71 | VERIFY_NONE | here |
-| Httparty.rb:7:1:7:55 | call to get | This request may run with $@. | Httparty.rb:7:37:7:54 | Pair | certificate validation disabled |
+| HttpClient.rb:6:1:6:33 | call to get | This request may run without certificate validation because it is $@. | HttpClient.rb:5:33:5:57 | ... = ... | disabled here | HttpClient.rb:5:33:5:57 | VERIFY_NONE | here |
-| Httparty.rb:10:1:10:59 | call to get | This request may run with $@. | Httparty.rb:10:39:10:56 | Pair | certificate validation disabled |
+| Httparty.rb:4:1:4:50 | call to get | This request may run without certificate validation because it is $@. | Httparty.rb:4:45:4:49 | false | disabled here | Httparty.rb:4:45:4:49 | false | here |
-| Httparty.rb:13:1:13:70 | call to post | This request may run with $@. | Httparty.rb:13:57:13:69 | Pair | certificate validation disabled |
+| Httparty.rb:7:1:7:55 | call to get | This request may run without certificate validation because it is $@. | Httparty.rb:7:50:7:54 | false | disabled here | Httparty.rb:7:50:7:54 | false | here |
-| Httparty.rb:16:1:16:74 | call to post | This request may run with $@. | Httparty.rb:16:59:16:71 | Pair | certificate validation disabled |
+| Httparty.rb:10:1:10:59 | call to get | This request may run without certificate validation because it is $@. | Httparty.rb:10:52:10:56 | false | disabled here | Httparty.rb:10:52:10:56 | false | here |
-| NetHttp.rb:9:12:9:31 | call to request | This request may run with $@. | NetHttp.rb:7:20:7:44 | ... = ... | certificate validation disabled |
+| Httparty.rb:13:1:13:70 | call to post | This request may run without certificate validation because it is $@. | Httparty.rb:13:65:13:69 | false | disabled here | Httparty.rb:13:65:13:69 | false | here |
-| OpenURI.rb:4:1:4:78 | call to open | This request may run with $@. | OpenURI.rb:4:36:4:77 | Pair | certificate validation disabled |
+| Httparty.rb:16:1:16:74 | call to post | This request may run without certificate validation because it is $@. | Httparty.rb:16:67:16:71 | false | disabled here | Httparty.rb:16:67:16:71 | false | here |
-| OpenURI.rb:7:1:7:82 | call to open | This request may run with $@. | OpenURI.rb:7:38:7:79 | Pair | certificate validation disabled |
+| NetHttp.rb:9:12:9:31 | call to request | This request may run without certificate validation because it is $@. | NetHttp.rb:7:20:7:44 | ... = ... | disabled here | NetHttp.rb:7:20:7:44 | VERIFY_NONE | here |
-| OpenURI.rb:11:1:11:43 | call to open | This request may run with $@. | OpenURI.rb:10:13:10:54 | Pair | certificate validation disabled |
+| OpenURI.rb:4:1:4:78 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:4:53:4:77 | VERIFY_NONE | disabled here | OpenURI.rb:4:53:4:77 | VERIFY_NONE | here |
-| OpenURI.rb:14:1:14:81 | call to open | This request may run with $@. | OpenURI.rb:14:39:14:80 | Pair | certificate validation disabled |
+| OpenURI.rb:7:1:7:82 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:7:55:7:79 | VERIFY_NONE | disabled here | OpenURI.rb:7:55:7:79 | VERIFY_NONE | here |
-| OpenURI.rb:17:1:17:85 | call to open | This request may run with $@. | OpenURI.rb:17:41:17:82 | Pair | certificate validation disabled |
+| OpenURI.rb:11:1:11:43 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:10:30:10:54 | VERIFY_NONE | disabled here | OpenURI.rb:10:30:10:54 | VERIFY_NONE | here |
-| OpenURI.rb:21:1:21:46 | call to open | This request may run with $@. | OpenURI.rb:20:13:20:54 | Pair | certificate validation disabled |
+| OpenURI.rb:14:1:14:81 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:14:56:14:80 | VERIFY_NONE | disabled here | OpenURI.rb:14:56:14:80 | VERIFY_NONE | here |
-| RestClient.rb:5:12:5:23 | call to get | This request may run with $@. | RestClient.rb:4:60:4:96 | Pair | certificate validation disabled |
+| OpenURI.rb:17:1:17:85 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:17:58:17:82 | VERIFY_NONE | disabled here | OpenURI.rb:17:58:17:82 | VERIFY_NONE | here |
-| RestClient.rb:9:12:9:23 | call to get | This request may run with $@. | RestClient.rb:8:62:8:98 | Pair | certificate validation disabled |
+| OpenURI.rb:21:1:21:46 | call to open | This request may run without certificate validation because it is $@. | OpenURI.rb:20:30:20:54 | VERIFY_NONE | disabled here | OpenURI.rb:20:30:20:54 | VERIFY_NONE | here |
-| RestClient.rb:14:12:14:23 | call to get | This request may run with $@. | RestClient.rb:12:13:12:49 | Pair | certificate validation disabled |
+| RestClient.rb:5:12:5:23 | call to get | This request may run without certificate validation because it is $@. | RestClient.rb:4:72:4:96 | VERIFY_NONE | disabled here | RestClient.rb:4:72:4:96 | VERIFY_NONE | here |
-| Typhoeus.rb:4:1:4:62 | call to get | This request may run with $@. | Typhoeus.rb:4:41:4:61 | Pair | certificate validation disabled |
+| RestClient.rb:9:12:9:23 | call to get | This request may run without certificate validation because it is $@. | RestClient.rb:8:74:8:98 | VERIFY_NONE | disabled here | RestClient.rb:8:74:8:98 | VERIFY_NONE | here |
-| Typhoeus.rb:8:1:8:54 | call to post | This request may run with $@. | Typhoeus.rb:7:37:7:57 | Pair | certificate validation disabled |
+| RestClient.rb:14:12:14:23 | call to get | This request may run without certificate validation because it is $@. | RestClient.rb:12:25:12:49 | VERIFY_NONE | disabled here | RestClient.rb:12:25:12:49 | VERIFY_NONE | here |
+| RestClient.rb:19:12:19:23 | call to get | This request may run without certificate validation because it is $@ by the value from $@. | RestClient.rb:18:72:18:76 | value | disabled here | RestClient.rb:17:9:17:33 | VERIFY_NONE | here |
+| Typhoeus.rb:4:1:4:62 | call to get | This request may run without certificate validation because it is $@. | Typhoeus.rb:4:57:4:61 | false | disabled here | Typhoeus.rb:4:57:4:61 | false | here |
+| Typhoeus.rb:8:1:8:54 | call to post | This request may run without certificate validation because it is $@. | Typhoeus.rb:7:53:7:57 | false | disabled here | Typhoeus.rb:7:53:7:57 | false | here |

ruby/ql/test/query-tests/security/cwe-295/RestClient.rb

@@ -13,6 +13,11 @@ options = { verify_ssl: OpenSSL::SSL::VERIFY_NONE }
 resource = RestClient::Resource.new("https://example.com", options)
 response = resource.get
 
+# BAD
+value = OpenSSL::SSL::VERIFY_NONE
+resource = RestClient::Resource.new("https://example.com", verify_ssl: value)
+response = resource.get
+
 # GOOD
 RestClient.get("https://example.com")
 
@@ -23,11 +28,11 @@ response = resource.get
 # GOOD
 resource = RestClient::Resource.new("https://example.com", verify_ssl: OpenSSL::SSL::VERIFY_PEER)
 response = resource.get
-# BAD
+# GOOD
 resource = RestClient::Resource.new("https://example.com", { verify_ssl: OpenSSL::SSL::VERIFY_PEER })
 response = resource.get
 
 # GOOD
 options = { verify_ssl: OpenSSL::SSL::VERIFY_PEER }
 resource = RestClient::Resource.new("https://example.com", options)
-response = resource.get
+response = resource.get