Merge pull request #14033 from hmac/excon-bugfix

Ruby: Fix bug in excon model
github · Aug 23, 2023 · 54c2221 · 54c2221
2 parents d146514 + d18ca3f
commit 54c2221
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 4 deletions.
diff --git a/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll b/ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll
@@ -64,10 +64,8 @@ class ExconHttpRequest extends Http::Client::Request::Range, DataFlow::CallNode
 
   /** Gets the value that controls certificate validation, if any. */
   DataFlow::Node getCertificateValidationControllingValue() {
-    exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
-      // Check for `ssl_verify_peer: false`
-      result = newCall.getKeywordArgumentIncludeHashArgument("ssl_verify_peer")
-    )
+    result =
+      connectionUse.(DataFlow::CallNode).getKeywordArgumentIncludeHashArgument("ssl_verify_peer")
   }
 
   cached

diff --git a/ruby/ql/test/query-tests/security/cwe-295/Excon.rb b/ruby/ql/test/query-tests/security/cwe-295/Excon.rb
@@ -46,4 +46,23 @@ def method8
   # GOOD
   Excon.defaults[:ssl_verify_peer] = false
   Excon.new("http://example.com/", ssl_verify_peer: true)
+end
+
+# Regression test for excon
+
+class Excon
+  def self.new(params)
+    Excon::Connection.new(params)
+  end
+end
+
+def method9
+  # GOOD: connection is not used
+  Excon.new("foo", ssl_verify_peer: false)
+end
+
+def method10
+  # GOOD
+  connection = Excon.new("foo")
+  connection.get("bar")
 end