Block cross-origin redirects by default on 1.4

.rubocop_gradual.lock

@@ -2,8 +2,8 @@
   "bin/bundle:3474730829": [
     [66, 5, 20, "ThreadSafety/ClassInstanceVariable: Avoid class instance variables.", 2485198147]
   ],
-  "lib/oauth2/client.rb:1044992130": [
-    [167, 16, 12, "Style/HashConversion: Prefer `ary.to_h` to `Hash[ary]`.", 4245087757]
+  "lib/oauth2/client.rb:2571398303": [
+    [169, 16, 12, "Style/HashConversion: Prefer `ary.to_h` to `Hash[ary]`.", 4245087757]
   ],
   "oauth2.gemspec:4057149467": [
     [127, 3, 53, "Gemspec/DependencyVersion: Dependency version specification is required.", 1845662118],
@@ -20,17 +20,17 @@
     [42, 15, 33, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 297534737],
     [51, 15, 38, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1480816240]
   ],
-  "spec/oauth2/client_spec.rb:626162787": [
+  "spec/oauth2/client_spec.rb:2423960917": [
     [158, 7, 467, "RSpec/NoExpectationExample: No expectation found in this example.", 1846255259],
     [177, 7, 570, "RSpec/NoExpectationExample: No expectation found in this example.", 354025061],
     [190, 15, 20, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2320605227],
     [205, 15, 20, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1276531672],
     [220, 15, 43, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1383956904],
     [235, 15, 43, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3376202107],
-    [458, 5, 369, "RSpec/NoExpectationExample: No expectation found in this example.", 648287915],
-    [467, 5, 471, "RSpec/NoExpectationExample: No expectation found in this example.", 1581534062],
-    [539, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785],
-    [547, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785],
-    [582, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785]
+    [494, 5, 369, "RSpec/NoExpectationExample: No expectation found in this example.", 648287915],
+    [503, 5, 471, "RSpec/NoExpectationExample: No expectation found in this example.", 1581534062],
+    [575, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785],
+    [583, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785],
+    [618, 45, 2, "RSpec/BeEq: Prefer `be` over `eq`.", 5860785]
   ]
 }

CHANGELOG.md

@@ -119,7 +119,7 @@ Please file a bug if you notice a violation of semantic versioning.
 ### Security
 
 - Prevent protocol-relative redirects from sending bearer tokens to an attacker-controlled host ([GHSA-pp92-crg2-gfv9](https://github.com/ruby-oauth/oauth2/security/advisories/GHSA-pp92-crg2-gfv9)).
-- Remove authorization headers, request bodies, and query parameters from cross-origin redirects.
+- Block cross-origin redirects by default to prevent credential replay and attacker-supplied token responses ([GHSA-h4fh-g8xf-8255](https://github.com/ruby-oauth/oauth2/security/advisories/GHSA-h4fh-g8xf-8255)). Legacy cross-origin redirects can be explicitly enabled with `:allow_cross_origin_redirects => true`.
 - Redact access tokens, refresh tokens, client secrets, authorization codes, and other sensitive fields from opt-in debug logs.
 
 ## [1.4.11] - 2022-09-16

README.md

@@ -368,6 +368,12 @@ For more see [SECURITY.md][🚎sec-pol].
 
 ## ⚙️ Configuration
 
+Cross-origin redirects are blocked by default so OAuth credentials are not sent
+to another origin and a redirected server cannot supply a token response. To
+restore legacy cross-origin redirect behavior for a trusted provider, set
+`:allow_cross_origin_redirects => true` when creating the client. Enabling this
+option accepts the security risks of following a redirect to another origin.
+
 ## 🔧 Basic Usage
 
 ## 🦷 FLOSS Funding

lib/oauth2/client.rb

@@ -29,6 +29,7 @@ class Client # rubocop:disable Metrics/ClassLength
     # @option options [Symbol] :auth_scheme (:basic_auth) HTTP method to use to authorize request (:basic_auth or :request_body)
     # @option options [Hash] :connection_opts ({}) Hash of connection options to pass to initialize Faraday with
     # @option options [FixNum] :max_redirects (5) maximum number of redirects to follow
+    # @option options [Boolean] :allow_cross_origin_redirects (false) whether to follow redirects to a different origin
     # @option options [Boolean] :raise_errors (true) whether or not to raise an OAuth2::Error on responses with 400+ status codes
     # @option options [Logger] :logger (::Logger.new($stdout)) which logger to use when OAUTH_DEBUG is enabled
     # @option options [Proc] (DEPRECATED) :extract_access_token  proc that extracts the access token from the response
@@ -46,6 +47,7 @@ def initialize(client_id, client_secret, options = {}, &block)
                   :connection_opts => {},
                   :connection_build => block,
                   :max_redirects => 5,
+                  :allow_cross_origin_redirects => false,
                   :raise_errors => true,
                   :extract_access_token => DEFAULT_EXTRACT_ACCESS_TOKEN, # DEPRECATED
                   :logger => ::Logger.new($stdout)}.merge(opts)
@@ -296,9 +298,22 @@ def build_access_token(response, access_token_opts, extract_access_token)
     def sanitize_redirect_options(opts, current_url, next_url)
       current_uri = URI.parse(current_url)
       next_uri = URI.parse(next_url)
-      return opts if current_uri.scheme == next_uri.scheme &&
+      same_origin = current_uri.scheme == next_uri.scheme &&
         current_uri.host == next_uri.host &&
         current_uri.port == next_uri.port
+      return opts if same_origin
+
+      unless options[:allow_cross_origin_redirects]
+        blocked_response = Struct.new(:body, :headers, :status).new(
+          MultiJson.encode(
+            "error" => "cross_origin_redirect_blocked",
+            "error_description" => "Cross-origin redirect blocked. Set :allow_cross_origin_redirects to true to allow it."
+          ),
+          {"Content-Type" => "application/json"},
+          0
+        )
+        raise Error.new(Response.new(blocked_response))
+      end
 
       safe_opts = opts.dup
       [:body, "body", :params, "params"].each { |key| safe_opts.delete(key) }

spec/oauth2/client_spec.rb

@@ -279,8 +279,44 @@
       expect(response.body).to eq("api.example.com")
     end
 
-    it "does not forward credentials across cross-origin redirects" do
+    it "blocks cross-origin redirects by default" do
+      redirected = false
       client = described_class.new("abc", "def", :site => "https://api.example.com") do |builder|
+        builder.adapter :test do |stub|
+          stub.post("/start") do
+            [307, {"Content-Type" => "text/plain", "location" => "https://attacker.example/collect"}, ""]
+          end
+          stub.post("/collect") do |env|
+            redirected = true
+            [200, {}, MultiJson.encode(
+              :body => env[:body],
+              :params => env[:params],
+              :authorization => env[:request_headers]["Authorization"]
+            )]
+          end
+        end
+      end
+
+      expect do
+        client.request(
+          :post,
+          "/start",
+          :body => "client_secret=secret",
+          :params => {"code" => "sensitive-code"},
+          :headers => {"Authorization" => "Bearer sensitive-token"}
+        )
+      end.to raise_error(OAuth2::Error) { |error| expect(error.code).to eq("cross_origin_redirect_blocked") }
+
+      expect(redirected).to be(false)
+    end
+
+    it "strips per-request credentials when cross-origin redirects are explicitly allowed" do
+      client = described_class.new(
+        "abc",
+        "def",
+        :site => "https://api.example.com",
+        :allow_cross_origin_redirects => true
+      ) do |builder|
         builder.adapter :test do |stub|
           stub.post("/start") do
             [307, {"Content-Type" => "text/plain", "location" => "https://attacker.example/collect"}, ""]