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

HTTPS Upgrades should prevent referrer stripping on same site #28809

Closed
arthuredelstein opened this issue Feb 28, 2023 · 7 comments · Fixed by brave/brave-core#17856
Closed

Comments

@arthuredelstein
Copy link

arthuredelstein commented Feb 28, 2023

When the referrer policy is strict-origin-when-cross-origin, clicking from a secure page to a same-site insecure link should result in stripping the path from the referrer. However, if HTTPS by Default (or HTTPS-Only Mode) upgrades navigation to HTTPS, then we effectively have a same-origin navigation and the referrer should not be stripped. Unfortunately it looks HTTPS-Only Mode (and therefore HTTPS by Default) are allowing the referrer to be stripped because of the way they use an internal dummy redirect.

Steps to reproduce:

  1. Use devtools to use a mobile UA (I used Android (4.0.2) browser.
  2. Visit https://anicobin.ldblog.jp/archives/59981055.html
  3. Click on the comments button:
    image

Expected result
A page with comments should be loaded:

image

Actual result
A "load error" page will be displayed:
image

Demo at https://gif.fanboy.co.nz/gif/anicobin.gif

@arthuredelstein arthuredelstein added OS/Android Fixes related to Android browser functionality OS/Desktop labels Feb 28, 2023
@arthuredelstein arthuredelstein self-assigned this Feb 28, 2023
@diracdeltas
Copy link
Member

However, if HTTPS by Default (or HTTPS-Only Mode) upgrades navigation to HTTPS, then we effectively have a same-origin navigation and the referrer should not be stripped.

what happens if the navigation target is HTTP but gets upgraded through HSTS? does the same bug occur?

@arthuredelstein
Copy link
Author

However, if HTTPS by Default (or HTTPS-Only Mode) upgrades navigation to HTTPS, then we effectively have a same-origin navigation and the referrer should not be stripped.

what happens if the navigation target is HTTP but gets upgraded through HSTS? does the same bug occur?

No, it does not occur in that case.

@arthuredelstein
Copy link
Author

I confirmed that this bug was fixed by brave/brave-core#17856

@kjozwiak
Copy link
Member

Added this issue into brave/brave-core#17856 (comment) & brave/brave-core#18179 (comment) so it can also be verified. Also adding missing labels/milestones.

@kjozwiak
Copy link
Member

The above requires 1.51.107 or higher for 1.51.x verification 👍

@stephendonner
Copy link

Verified PASSED using

Brave 1.51.107 Chromium: 113.0.5672.63 (Official Build) (x86_64)
Revision 0e1a4471d5ae5bf128b1bd8f4d627c8cbd55f70c-refs/branch-heads/5672@{#912}
OS macOS Version 13.4 (Build 22F5049e)

Followed the steps in this issue, as reported

Confirmed I could load & read the comments section on https://anicobin.ldblog.jp/archives/59981055.html

example example example example
Screenshot 2023-04-29 at 3 39 47 AM Screenshot 2023-04-29 at 3 40 07 AM Screenshot 2023-04-29 at 3 40 18 AM Screenshot 2023-04-29 at 3 46 22 AM

@kjozwiak
Copy link
Member

kjozwiak commented May 1, 2023

Verification PASSED on Pixel 6 running Android 14 using the following build(s):

Brave | 1.53.1 Chromium: 113.0.5672.53 (Official Build) canary (32-bit)
--- | ---
Revision | 12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}
OS | Android 13; Build/UPB1.230309.014; 33; UpsideDownCake

Using the STR/Cases outlined via #28809 (comment), ensured the following:

Example Example Example
Screenshot_20230501-002616 Screenshot_20230501-002632 Screenshot_20230501-002638

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

Successfully merging a pull request may close this issue.

5 participants