Merge pull request #10913 from thiggy1342/expand-ruby-ssrf-sinks-faraday-connection-new

Ruby: Add Faraday::Connection.new as sink for SSRF query

Author: hmac

ruby/ql/lib/change-notes/2022-10-20-expand-faraday-model-for-ssrf-sink.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+- Instantiations using `Faraday::Connection.new` are now recognized as part of `FaradayHttpRequest`s, meaning they will be considered as sinks for queries such as `rb/request-forgery`.

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

@@ -34,7 +34,9 @@ class FaradayHttpRequest extends Http::Client::Request::Range, DataFlow::CallNod
         // one-off requests
         API::getTopLevelMember("Faraday"),
         // connection re-use
-        API::getTopLevelMember("Faraday").getInstance()
+        API::getTopLevelMember("Faraday").getInstance(),
+        // connection re-use with Faraday::Connection.new instantiation
+        API::getTopLevelMember("Faraday").getMember("Connection").getInstance()
       ] and
     requestNode =
       connectionNode

ruby/ql/test/library-tests/frameworks/http_clients/Faraday.rb

@@ -33,4 +33,8 @@
 
 connection = Faraday.new(url: "http://example.com")
 resp11 = connection.get("/")
-resp11.body
+resp11.body
+
+connection = Faraday::Connection.new(url: "https://example.com")
+resp12 = connection.get("/")
+resp12.body

ruby/ql/test/library-tests/frameworks/http_clients/HttpClients.expected

@@ -28,6 +28,9 @@
 | Faraday.rb:35:10:35:28 | call to get | Faraday | Faraday.rb:34:26:34:50 | Pair | Faraday.rb:36:1:36:11 | call to body |
 | Faraday.rb:35:10:35:28 | call to get | Faraday | Faraday.rb:34:31:34:50 | "http://example.com" | Faraday.rb:36:1:36:11 | call to body |
 | Faraday.rb:35:10:35:28 | call to get | Faraday | Faraday.rb:35:25:35:27 | "/" | Faraday.rb:36:1:36:11 | call to body |
+| Faraday.rb:39:10:39:28 | call to get | Faraday | Faraday.rb:38:38:38:63 | Pair | Faraday.rb:40:1:40:11 | call to body |
+| Faraday.rb:39:10:39:28 | call to get | Faraday | Faraday.rb:38:43:38:63 | "https://example.com" | Faraday.rb:40:1:40:11 | call to body |
+| Faraday.rb:39:10:39:28 | call to get | Faraday | Faraday.rb:39:25:39:27 | "/" | Faraday.rb:40:1:40:11 | call to body |
 | HttpClient.rb:3:9:3:45 | call to get | HTTPClient | HttpClient.rb:3:24:3:44 | "http://example.com/" | HttpClient.rb:4:1:4:10 | call to body |
 | HttpClient.rb:6:9:6:65 | call to post | HTTPClient | HttpClient.rb:6:25:6:45 | "http://example.com/" | HttpClient.rb:7:1:7:13 | call to content |
 | HttpClient.rb:9:9:9:64 | call to put | HTTPClient | HttpClient.rb:9:24:9:44 | "http://example.com/" | HttpClient.rb:10:1:10:15 | call to http_body |

ruby/ql/test/query-tests/security/cwe-918/ServerSideRequestForgery.expected

@@ -1,10 +1,18 @@
 edges
-| ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | ServerSideRequestForgery.rb:9:32:9:60 | ...[...] :  |
-| ServerSideRequestForgery.rb:9:32:9:60 | ...[...] :  | ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" |
+| ServerSideRequestForgery.rb:10:32:10:37 | call to params :  | ServerSideRequestForgery.rb:10:32:10:60 | ...[...] :  |
+| ServerSideRequestForgery.rb:10:32:10:60 | ...[...] :  | ServerSideRequestForgery.rb:11:31:11:62 | "#{...}/logins" |
+| ServerSideRequestForgery.rb:15:33:15:38 | call to params :  | ServerSideRequestForgery.rb:15:33:15:44 | ...[...] |
+| ServerSideRequestForgery.rb:20:45:20:50 | call to params :  | ServerSideRequestForgery.rb:20:45:20:56 | ...[...] |
 nodes
-| ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | semmle.label | call to params :  |
-| ServerSideRequestForgery.rb:9:32:9:60 | ...[...] :  | semmle.label | ...[...] :  |
-| ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | semmle.label | "#{...}/logins" |
+| ServerSideRequestForgery.rb:10:32:10:37 | call to params :  | semmle.label | call to params :  |
+| ServerSideRequestForgery.rb:10:32:10:60 | ...[...] :  | semmle.label | ...[...] :  |
+| ServerSideRequestForgery.rb:11:31:11:62 | "#{...}/logins" | semmle.label | "#{...}/logins" |
+| ServerSideRequestForgery.rb:15:33:15:38 | call to params :  | semmle.label | call to params :  |
+| ServerSideRequestForgery.rb:15:33:15:44 | ...[...] | semmle.label | ...[...] |
+| ServerSideRequestForgery.rb:20:45:20:50 | call to params :  | semmle.label | call to params :  |
+| ServerSideRequestForgery.rb:20:45:20:56 | ...[...] | semmle.label | ...[...] |
 subpaths
 #select
-| ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | ServerSideRequestForgery.rb:9:32:9:37 | call to params :  | ServerSideRequestForgery.rb:10:31:10:62 | "#{...}/logins" | The URL of this request depends on a $@. | ServerSideRequestForgery.rb:9:32:9:37 | call to params | user-provided value |
+| ServerSideRequestForgery.rb:11:31:11:62 | "#{...}/logins" | ServerSideRequestForgery.rb:10:32:10:37 | call to params :  | ServerSideRequestForgery.rb:11:31:11:62 | "#{...}/logins" | The URL of this request depends on a $@. | ServerSideRequestForgery.rb:10:32:10:37 | call to params | user-provided value |
+| ServerSideRequestForgery.rb:15:33:15:44 | ...[...] | ServerSideRequestForgery.rb:15:33:15:38 | call to params :  | ServerSideRequestForgery.rb:15:33:15:44 | ...[...] | The URL of this request depends on a $@. | ServerSideRequestForgery.rb:15:33:15:38 | call to params | user-provided value |
+| ServerSideRequestForgery.rb:20:45:20:56 | ...[...] | ServerSideRequestForgery.rb:20:45:20:50 | call to params :  | ServerSideRequestForgery.rb:20:45:20:56 | ...[...] | The URL of this request depends on a $@. | ServerSideRequestForgery.rb:20:45:20:50 | call to params | user-provided value |

ruby/ql/test/query-tests/security/cwe-918/ServerSideRequestForgery.rb

@@ -1,4 +1,5 @@
 require "excon"
+require "faraday"
 require "json"
 
 class PostsController < ActionController::Base
@@ -10,6 +11,16 @@ def create
         response = Excon.post("#{users_service_domain}/logins", body: {user_id: user}).body
         token = JSON.parse(response)["token"]
 
+        # BAD - user can control the entire URL for the request using Faraday library
+        conn = Faraday.new(url: params[:url])
+        resp = conn.post
+        token = JSON.parse(resp)["token"]
+
+        # BAD - user can control the entire URL for the request using Faraday::Connection library
+        conn = Faraday::Connection.new(url: params[:url])
+        resp = conn.post
+        token = JSON.parse(resp)["token"]
+
         # GOOD - user can only control the suffix of the URL
         users_service_path = params[:users_service_path]
         response = Excon.post("users-service/#{users_service_path}", body: {user_id: user}).body