Add permitted_hosts setting

The Sinatra project received a security report with the following details: > Title: Reliance on Untrusted Inputs in a Security Decision > CWE ID: CWE-807 > CVE ID: CVE-2024-21510 > Credit: t0rchwo0d > Description: The sinatra package is vulnerable to Reliance on Untrusted > Inputs in a Security Decision via the `X-Forwarded-Host (XFH)` header. > When making a request to a method with redirect applied, it is possible > to trigger an Open Redirect Attack by inserting an arbitrary address > into this header. If used for caching purposes, such as with servers > like Nginx, or as a reverse proxy, without handling the > `X-Forwarded-Host` header, attackers can potentially exploit Cache > Poisoning or Routing-based SSRF. The vulnerable code was introduced in fae7c01. Sinatra can not know whether the header value can be trusted or not without input from the app creator. This change introduce the `permitted_hosts` setting for that. It is implemented as a Rack middleware, bundled with rack-protection, but not exposed as a default nor opt-in protection. It is meant to be used by itself, as sharing reaction with other protections is not ideal, see #2012.
sinatra · Nov 8, 2024 · 691a573 · 691a573
1 parent 3c888f7
commit 691a573
Show file tree

Hide file tree

Showing 8 changed files with 290 additions and 6 deletions.
diff --git a/README.md b/README.md
@@ -2014,6 +2014,16 @@ set :protection, :session => true
     paths.
   </dd>
 
+  <dt>permitted_hosts</dt>
+    <dd>
+      An array of hostnames your app recognizes. Requests with an unrecognized hostname
+      will be stopped to prevent DNS rebinding and other host header attacks.
+      Checks the <tt>Host</tt>, <tt>X-Forwarded-Host</tt> and <tt>Forwarded</tt> headers.
+    </dd>
+    <dd>
+      By default the array is empty and all hostnames are permitted.
+    </dd>
+
   <dt>port</dt>
     <dd>Port to listen on. Only used for built-in server.</dd>
 

diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb
@@ -63,7 +63,7 @@ def preferred_type(*types)
     alias secure? ssl?
 
     def forwarded?
-      @env.include? 'HTTP_X_FORWARDED_HOST'
+      Rack::Protection::HostAuthorization.forwarded?(self)
     end
 
     def safe?
@@ -330,11 +330,7 @@ def uri(addr = nil, absolute = true, add_script_name = true)
       uri = [host = String.new]
       if absolute
         host << "http#{'s' if request.secure?}://"
-        host << if request.forwarded? || (request.port != (request.secure? ? 443 : 80))
-                  request.host_with_port
-                else
-                  request.host
-                end
+        host << Rack::Protection::HostAuthorization.host_from(request: request)
       end
       uri << request.script_name.to_s if add_script_name
       uri << (addr || request.path_info).to_s
@@ -1821,6 +1817,7 @@ def setup_default_middleware(builder)
         setup_logging    builder
         setup_sessions   builder
         setup_protection builder
+        setup_host_authorization builder
       end
 
       def setup_middleware(builder)
@@ -1869,6 +1866,11 @@ def setup_protection(builder)
         builder.use Rack::Protection, options
       end
 
+      def setup_host_authorization(builder)
+        options = { permitted_hosts: permitted_hosts }
+        builder.use Rack::Protection::HostAuthorization, options
+      end
+
       def setup_sessions(builder)
         return unless sessions?
 
@@ -1934,6 +1936,7 @@ def force_encoding(*args)
     set :sessions, false
     set :session_store, Rack::Session::Cookie
     set :logging, false
+    set :permitted_hosts, []
     set :protection, true
     set :method_override, false
     set :use_code, false

diff --git a/rack-protection/lib/rack/protection.rb b/rack-protection/lib/rack/protection.rb
@@ -12,6 +12,7 @@ module Protection
     autoload :EscapedParams,         'rack/protection/escaped_params'
     autoload :FormToken,             'rack/protection/form_token'
     autoload :FrameOptions,          'rack/protection/frame_options'
+    autoload :HostAuthorization,     'rack/protection/host_authorization'
     autoload :HttpOrigin,            'rack/protection/http_origin'
     autoload :IPSpoofing,            'rack/protection/ip_spoofing'
     autoload :JsonCsrf,              'rack/protection/json_csrf'

diff --git a/rack-protection/lib/rack/protection/host_authorization.rb b/rack-protection/lib/rack/protection/host_authorization.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'rack/protection'
+
+module Rack
+  module Protection
+    ##
+    # Prevented attack::   DNS rebinding and other Host header attacks
+    # Supported browsers:: all
+    # More infos::         https://en.wikipedia.org/wiki/DNS_rebinding
+    #                      https://portswigger.net/web-security/host-header
+    #
+    # Blocks HTTP requests with an unrecognized hostname in any of the following
+    # HTTP headers: Host, X-Forwarded-Host, Forwarded
+    #
+    # If you want to permit a specific hostname, you can pass in as the `:permitted_hosts` option:
+    #
+    #     use Rack::Protection::HostAuthorization, permitted_hosts: ["www.example.org", "sinatrarb.com"]
+    #
+    # The `:allow_if` option can also be set to a proc to use custom allow/deny logic.
+    class HostAuthorization < Base
+      default_reaction :deny
+      default_options allow_if: nil,
+                      message: "Host not permitted"
+
+      def self.forwarded?(request)
+        request.get_header(Request::HTTP_X_FORWARDED_HOST)
+      end
+
+      def self.host_from(request:)
+        if forwarded?(request) || (request.port != (request.ssl? ? 443 : 80))
+          request.host_with_port
+        else
+          request.host
+        end
+      end
+
+      def initialize(*)
+        super
+        @permitted_hosts = Array(options[:permitted_hosts]).map(&:downcase)
+      end
+
+      def accepts?(env)
+        return true if options[:allow_if]&.call(env)
+        return true if @permitted_hosts.empty?
+
+        request = Request.new(env)
+        origin_host = self.class.host_from(request: request)
+
+        @permitted_hosts.include?(origin_host.downcase)
+      end
+    end
+  end
+end
diff --git a/rack-protection/spec/lib/rack/protection/host_authorization_spec.rb b/rack-protection/spec/lib/rack/protection/host_authorization_spec.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+RSpec.describe Rack::Protection::HostAuthorization do
+  it_behaves_like 'any rack application'
+
+  def assert_response(outcome:, headers:, last_response:)
+    fail_message = "Expected outcome '#{outcome}' for headers '#{headers}' " \
+                   "last_response.status '#{last_response.status}'"
+
+    case outcome
+    when :allowed
+      expect(last_response).to be_ok, fail_message
+    when :stopped
+      expect(last_response.status).to eq(403), fail_message
+      expect(last_response.body).to eq("Host not permitted"), fail_message
+    end
+  end
+
+  allowed_host = "example.com"
+  bad_host = "evil.com"
+  test_cases = [
+    # good requests
+    [:allowed, { "HTTP_HOST" => allowed_host }],
+    [:allowed, { "HTTP_X_FORWARDED_HOST" => allowed_host }],
+    [:allowed, { "HTTP_FORWARDED" => "host=#{allowed_host}" }],
+
+    # bad requests
+    [:stopped, { "HTTP_HOST" => bad_host }],
+    [:stopped, { "HTTP_X_FORWARDED_HOST" => bad_host }],
+    [:stopped, { "HTTP_FORWARDED" => "host=#{bad_host}" }],
+    [:stopped, { "HTTP_HOST" => allowed_host, "HTTP_X_FORWARDED_HOST" => bad_host }],
+    [:stopped, { "HTTP_HOST" => allowed_host, "HTTP_FORWARDED" => "host=#{bad_host}" }],
+  ]
+
+  test_cases.each do |outcome, headers|
+    it 'allows/stops requests based on the permitted hosts specified' do
+      mock_app do
+        use Rack::Protection::HostAuthorization, permitted_hosts: [allowed_host]
+        run DummyApp
+      end
+
+      get("/", {}, headers)
+
+      assert_response(outcome: outcome, headers: headers, last_response: last_response)
+    end
+  end
+
+  it "accepts requests for non-permitted hosts when allow_if is true" do
+    mock_app do
+      use Rack::Protection::HostAuthorization, allow_if: ->(_env) { true },
+                                               permitted_hosts: [allowed_host]
+      run DummyApp
+    end
+
+    get("/", {}, "HTTP_HOST" => bad_host)
+
+    expect(last_response).to be_ok
+  end
+
+  it "allows the response given for non-permitted requests to be customized" do
+    message = "Unrecognized host"
+    mock_app do
+      use Rack::Protection::HostAuthorization, message: message, status: 406,
+                                               permitted_hosts: [allowed_host]
+      run DummyApp
+    end
+
+    get("/", {}, "HTTP_HOST" => bad_host)
+
+    expect(last_response.status).to eq(406)
+    expect(last_response.body).to eq(message)
+  end
+
+  describe "when the header value is upcased but the permitted host not" do
+    allowed_host = "example.com"
+    host_in_request = allowed_host.upcase
+    test_cases = [
+      { "HTTP_HOST" => host_in_request },
+      { "HTTP_X_FORWARDED_HOST" => host_in_request },
+      { "HTTP_FORWARDED" => "host=#{host_in_request}" },
+    ]
+
+    test_cases.each do |headers|
+      it "works" do
+        mock_app do
+          use Rack::Protection::HostAuthorization, permitted_hosts: [allowed_host]
+          run DummyApp
+        end
+
+        get("/", {}, headers)
+
+        expect(last_response).to be_ok
+      end
+    end
+  end
+
+  describe "when the permitted host is upcased but the header value is not" do
+    allowed_host = "example.com".upcase
+    host_in_request = allowed_host
+    test_cases = [
+      { "HTTP_HOST" => host_in_request },
+      { "HTTP_X_FORWARDED_HOST" => host_in_request },
+      { "HTTP_FORWARDED" => "host=#{host_in_request}" },
+    ]
+
+    test_cases.each do |headers|
+      it "works" do
+        mock_app do
+          use Rack::Protection::HostAuthorization, permitted_hosts: [allowed_host]
+          run DummyApp
+        end
+
+        get("/", {}, headers)
+
+        expect(last_response).to be_ok
+      end
+    end
+  end
+end
diff --git a/test/helpers_test.rb b/test/helpers_test.rb
@@ -258,6 +258,32 @@ def status_app(code, &block)
       assert_equal 'http://example.com/foo', response['Location']
     end
 
+    it 'redirects to permitted hosts' do
+      mock_app do
+        set :permitted_hosts, ['example.com']
+
+        get('/') { redirect '/foo' }
+      end
+
+      request = Rack::MockRequest.new(@app)
+      response = request.get('/', 'HTTP_X_FORWARDED_HOST' => 'example.com')
+      assert_equal 302, response.status
+      assert_equal 'http://example.com/foo', response['Location']
+    end
+
+    it 'stops requests to non-permitted hosts' do
+      mock_app do
+        set :permitted_hosts, ['example.com']
+
+        get('/') { redirect '/foo' }
+      end
+
+      request = Rack::MockRequest.new(@app)
+      response = request.get('/', 'HTTP_X_FORWARDED_HOST' => 'evil.com')
+      assert_equal 403, response.status
+      assert_equal 'Host not permitted', response.body
+    end
+
     it 'accepts absolute URIs' do
       mock_app do
         get('/') do

diff --git a/test/host_authorization_test.rb b/test/host_authorization_test.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require_relative "test_helper"
+
+class HostAuthorization < Minitest::Test
+  def assert_response(outcome:, headers:, response:)
+    fail_message = "Expected outcome '#{outcome}' for headers '#{headers}'"
+
+    case outcome
+    when :allowed
+      assert_equal 200, response.status, fail_message
+      assert_equal "OK", response.body, fail_message
+    when :stopped
+      assert_equal 403, response.status, fail_message
+      assert_equal "Host not permitted", response.body, fail_message
+    end
+  end
+
+  allowed_host = "example.com"
+  bad_host = "evil.com"
+  test_cases = [
+    # good requests
+    [:allowed, { "HTTP_HOST" => allowed_host }],
+    [:allowed, { "HTTP_X_FORWARDED_HOST" => allowed_host }],
+    [:allowed, { "HTTP_FORWARDED" => "host=#{allowed_host}" }],
+
+    # bad requests
+    [:stopped, { "HTTP_HOST" => bad_host }],
+    [:stopped, { "HTTP_X_FORWARDED_HOST" => bad_host }],
+    [:stopped, { "HTTP_FORWARDED" => "host=#{bad_host}" }],
+    [:stopped, { "HTTP_HOST" => allowed_host, "HTTP_X_FORWARDED_HOST" => bad_host }],
+    [:stopped, { "HTTP_HOST" => allowed_host, "HTTP_FORWARDED" => "host=#{bad_host}" }],
+  ]
+
+  test_cases.each do |outcome, headers|
+    it "allows/stops requests based on the permitted hosts specified" do
+      mock_app do
+        set :permitted_hosts, [allowed_host]
+
+        get("/") { "OK" }
+      end
+
+      request = Rack::MockRequest.new(@app)
+      response = request.get("/", headers)
+
+      assert_response(outcome: outcome, headers: headers, response: response)
+    end
+  end
+
+  it "allows any requests when no permitted hosts are specified" do
+    test_cases.each do |_outcome, headers|
+      mock_app do
+        set :permitted_hosts, []
+
+        get("/") { "OK" }
+      end
+
+      fail_message = "Expected request with headers '#{headers}' to be allowed"
+      request = Rack::MockRequest.new(@app)
+      response = request.get("/", headers)
+
+      assert_equal 200, response.status, fail_message
+      assert_equal "OK", response.body, fail_message
+    end
+  end
+end
diff --git a/test/request_test.rb b/test/request_test.rb
@@ -36,6 +36,11 @@ class RequestTest < Minitest::Test
     assert !request.secure?
   end
 
+  it 'respects X-Forwarded-Host header' do
+    request = Sinatra::Request.new('HTTP_X_FORWARDED_HOST' => 'example.com')
+    assert request.forwarded?
+  end
+
   it 'respects X-Forwarded-Proto header for proxied SSL' do
     request = Sinatra::Request.new('HTTP_X_FORWARDED_PROTO' => 'https')
     assert request.secure?