From 92c45a0add2acab34e038b5dc52414fa148f7113 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 1 Jun 2020 16:56:29 +1000 Subject: [PATCH] fix: use relative URLs when base_url not explictly set to ensure app is not vulnerable to host header attacks --- lib/pact_broker/doc/controllers/app.rb | 6 +++++- .../ui/controllers/base_controller.rb | 2 +- lib/pact_broker/ui/views/index/_navbar.haml | 2 +- spec/integration/ui/index_spec.rb | 16 ++++++++++++++++ spec/lib/pact_broker/doc/controllers/app_spec.rb | 16 ++++++++++++++++ 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/lib/pact_broker/doc/controllers/app.rb b/lib/pact_broker/doc/controllers/app.rb index 600443b57..4b89a734d 100644 --- a/lib/pact_broker/doc/controllers/app.rb +++ b/lib/pact_broker/doc/controllers/app.rb @@ -46,7 +46,11 @@ def resource_exists? rel_name, context = nil private def base_url - PactBroker.configuration.base_url || request.base_url + # Using the X-Forwarded headers in the UI can leave the app vulnerable + # https://www.acunetix.com/blog/articles/automated-detection-of-host-header-attacks/ + # Either use the explicitly configured base url or an empty string, + # rather than request.base_url, which uses the X-Forwarded headers. + PactBroker.configuration.base_url || '' end end end diff --git a/lib/pact_broker/ui/controllers/base_controller.rb b/lib/pact_broker/ui/controllers/base_controller.rb index 192bedaa4..2a5b5c91f 100644 --- a/lib/pact_broker/ui/controllers/base_controller.rb +++ b/lib/pact_broker/ui/controllers/base_controller.rb @@ -12,7 +12,7 @@ class Base < Padrino::Application set :dump_errors, false # The padrino logger logs these for us. If this is enabled we get duplicate logging. def base_url - PactBroker.configuration.base_url || request.base_url + PactBroker.configuration.base_url || '' end end end diff --git a/lib/pact_broker/ui/views/index/_navbar.haml b/lib/pact_broker/ui/views/index/_navbar.haml index b4ed19eef..8a219d36e 100644 --- a/lib/pact_broker/ui/views/index/_navbar.haml +++ b/lib/pact_broker/ui/views/index/_navbar.haml @@ -7,7 +7,7 @@ %a{href: "#{base_url}?tags=true"} Show latest tags - else - %a{href: "#{base_url}"} + %a{href: "#{base_url}/"} Hide latest tags %a{href: "#{base_url}/hal-browser/browser.html"} diff --git a/spec/integration/ui/index_spec.rb b/spec/integration/ui/index_spec.rb index 7fa642f78..49d597342 100644 --- a/spec/integration/ui/index_spec.rb +++ b/spec/integration/ui/index_spec.rb @@ -32,5 +32,21 @@ expect(subject.body.scan('