Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Referrer spoofing could disable login CSRF protections on some sites #3422

Closed
fmarier opened this issue Feb 19, 2019 · 6 comments · Fixed by brave/brave-core#2260, brave/brave-core#1767 or brave/brave-core#2070
Assignees
Labels
feature/shields The overall Shields feature in Brave. priority/P3 The next thing for us to work on. It'll ride the trains. privacy/feature User-facing privacy- & security-focused feature work. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include security

Comments

@fmarier
Copy link
Member

fmarier commented Feb 19, 2019

Some sites (e.g. anything using the default Django authentication system) use the Referer header in order to prevent CSRF attacks on their login pages.

By setting the Referer to be the destination's origin (for cross-origin requests), we are essentially making sure that these security checks always pass, therefore making them ineffective at preventing CSRF attacks.

@diracdeltas suggested changing the referrer-spoofing code to:

  1. strip the referrer entirely on cross-origin navigation requests and iframe sub-resources,
  2. continue to spoof the referrer on cross-origin sub-resource requests, and
  3. continue to send the full referrer on all same-origin requests.
@fmarier fmarier added feature/shields The overall Shields feature in Brave. privacy/feature User-facing privacy- & security-focused feature work. labels Feb 19, 2019
@diracdeltas diracdeltas added security priority/P3 The next thing for us to work on. It'll ride the trains. labels Feb 19, 2019
@diracdeltas
Copy link
Member

If we go with 1, let's come up with a plan to QA it for new webcompatibility breakage.

@diracdeltas
Copy link
Member

looking at https://github.com/brave/brave-browser/wiki/Brave-Release-Schedule, if we got this change into a dev release, it seems that it would be in dev for ~6 weeks and beta for ~3 weeks before hitting the stable release channel. that might be good enough, depending on how many people are actively using dev/beta

@bbondy
Copy link
Member

bbondy commented Mar 24, 2019

Re-opening this because it was failing browser tests and had to be reverted.

14:34:27 [  FAILED  ] BraveContentBrowserClientReferrerTest.DefaultBehaviour, where TypeParam =  and GetParam() =  (658 ms)
14:34:27 [191/191] BraveContentBrowserClientReferrerTest.DefaultBehaviour (805 ms)
14:34:27 Retrying 1 test (retry #3)
14:34:28 [ RUN      ] BraveContentBrowserClientReferrerTest.DefaultBehaviour
14:34:28 [23530:23811:0324/073427.977652:ERROR:rewards_service_impl.cc(185)] Failed to read file: /private/var/folders/h4/yxf5y9nx2nb9mv3rqqyj61gw0000gp/T/.com.brave.Browser.5fL2Jv/dkJpr8T/Default/ledger_state
14:34:28 [23534:775:0324/073427.990706:ERROR:ledger_impl.cc(338)] Failed to initialize wallet
14:34:28 ../../brave/browser/brave_content_browser_client_browsertest.cc:493: Failure
14:34:28 Expected equality of these values:
14:34:28   referrer.url
14:34:28     Which is: 
14:34:28   kDocumentUrl
14:34:28     Which is: http://document.com/path?query
14:34:28 Stack trace:

The revert was merged here:
brave/brave-core#2049

@bbondy
Copy link
Member

bbondy commented Apr 6, 2019

Re-opening because it breaks playing youtube embed videos.
See this issue for steps to reproduce: #3988

@bbondy bbondy reopened this Apr 6, 2019
fmarier added a commit to brave/brave-core that referenced this issue Apr 10, 2019
fmarier added a commit to brave/brave-core that referenced this issue Apr 16, 2019
Fixes brave/brave-browser#3422.

This is based on the #2070 pull request which
was committed in 501f4e0 and
then reverted in 056ce15 because
of brave/brave-browser#3988.
fmarier added a commit to brave/brave-core that referenced this issue Apr 17, 2019
Fixes brave/brave-browser#3422.

This is based on the #2070 pull request which
was committed in 501f4e0 and
then reverted in 056ce15 because
of brave/brave-browser#3988.
fmarier added a commit to brave/brave-core that referenced this issue Apr 26, 2019
Fixes brave/brave-browser#3422.

This is based on the #2070 pull request which
was committed in 501f4e0 and
then reverted in 056ce15 because
of brave/brave-browser#3988.
@LaurenWags
Copy link
Member

LaurenWags commented Jul 11, 2019

Verified passed with

Brave 0.67.107 Chromium: 75.0.3770.100 (Official Build) beta(64-bit)
Revision cd0b15c8b6a4e70c44e27f35c37a4029bad3e3b0-refs/branch-heads/3770@{#1033}
OS Mac OS X

Verification passed on

Brave 0.67.106 Chromium: 75.0.3770.100 (Official Build) beta (64-bit)
Revision cd0b15c8b6a4e70c44e27f35c37a4029bad3e3b0-refs/branch-heads/3770@{#1033}
OS Windows 10 OS Version 1803 (Build 17134.523)

Verification passed on

Brave 0.67.110 Chromium: 75.0.3770.100 (Official Build) beta(64-bit)
Revision cd0b15c8b6a4e70c44e27f35c37a4029bad3e3b0-refs/branch-heads/3770@{#1033}
OS Ubuntu 18.04 LTS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment