Skip to content

Commit

Permalink
Ruby: Fix bug in excon model
Browse files Browse the repository at this point in the history
If a codebase included a definition for `Excon.new`, we matched
connection nodes to unrelated request nodes.
  • Loading branch information
hmac committed Aug 23, 2023
1 parent fb4b774 commit d18ca3f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
6 changes: 2 additions & 4 deletions ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions ruby/ql/test/query-tests/security/cwe-295/Excon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d18ca3f

Please sign in to comment.