brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.7k stars 2.31k forks source link

[hackerone] Restrict cosmetic filter creation to the origin of the page it's on #16863

Closed fmarier closed 1 year ago

fmarier commented 3 years ago

The content script which implements Shields cosmetic filtering talks to the extension background script over a postMessage channel. This means that a compromised renderer could send messages to the extension as well.

We should look into whether or not we can make use of a random token to restrict the messages accepted by the backend to those that originating from the content script injected by the extension.

Originally reported at https://hackerone.com/reports/1254125.

fmarier commented 3 years ago

Here's the pattern that iOS uses: https://github.com/brave/brave-ios/search?q=securityToken

diracdeltas commented 3 years ago

@keur have you looked at this one yet?

kkuehlz commented 3 years ago

@diracdeltas Not yet. Has no priority attached. We can put a P3 on this and I'll look into it this week?

Also might need to reach out to you or PJ to understand exactly what iOS is doing...

diracdeltas commented 3 years ago

P3 sounds good; added

fmarier commented 3 years ago

It is possible to insert arbitrary text into brave://adblock by changing the JS context to "Brave" in the devtools and running something like:

chrome.runtime.sendMessage({type:"cosmeticFilterCreate", host:"fmarier.org", selector:"body { background-color: red; }"})

This gets applied immediately in the page and results in the following rule in brave://adblock:

fmarier.org##body { background-color: red; }

but on a page reload it fails to parse:

Uncaught DOMException: Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule 'body { background-color: red; }{display:none !important;}'.

The fact that a compromise renderer can temporarily affect the contents/looks of a page is not a new capability and so that's not something we need to fix.

fmarier commented 3 years ago

I was however able to insert a rule for a different domain:

  1. Open http://example.com/.
  2. Open the devtools.
  3. Click on "top" (the JS context) in the Console tab and change it to "Brave".
  4. Run the following JS code: chrome.runtime.sendMessage({type:"cosmeticFilterCreate", host:"fmarier.org", selector:"#hidden-avatar"})
  5. Open https://fmarier.org and notice that the avatar photo is gone.
  6. Open brave://adblock/ to see that the following rule was inserted: fmarier.org###hidden-avatar

We should derive the host ourselves from sender.origin and remove the host parameter in order to limit such rule injections to just the current origin.

stephendonner commented 1 year ago

Verified PASSED using

Brave 1.50.91 Chromium: 111.0.5563.64 (Official Build) beta (x86_64)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS macOS Version 11.7.4 (Build 20G1120)

Steps:

  1. installed 1.50.91
  2. launched Brave
  3. loaded abcnews.com, cnn.com, and foxnews.com
  4. on each, context-clicked on their logo and chose "Brave-->Block element`
  5. selected the logo
  6. clicked on Create
  7. checked the filters in brave://settings/shields/filters
  8. clicked Reload this page, for each site
  9. ensured logos were still blocked
abcnews.com cnn.com foxnews.com brave://settings/shields/filters sites without logos
Screen Shot 2023-03-14 at 3 05 41 PM Screen Shot 2023-03-14 at 3 06 53 PM Screen Shot 2023-03-14 at 3 10 23 PM Screen Shot 2023-03-14 at 3 16 53 PM Screen Shot 2023-03-14 at 3 24 58 PM

Verification passed on

Brave 1.50.93 Chromium: 111.0.5563.64 (Official Build) beta (64-bit)
Revision c710e93d5b63b7095afe8c2c17df34408078439d-refs/branch-heads/5563@{#995}
OS Ubuntu 18.04 LTS

Steps:

  1. installed 1.50.x
  2. launched Brave
  3. loaded abcnews.com, cnn.com, and foxnews.com
  4. on each, context-clicked on their logo and chose "Brave-->Block element`
  5. selected the logo
  6. clicked on Create
  7. checked the filters in brave://settings/shields/filters
  8. clicked Reload this page, for each site
  9. ensured logos were still blocked
`interia.pl` ![image](https://user-images.githubusercontent.com/34715963/226595408-de0d1283-97b2-4cd6-a98b-f3c486611f37.png) ![image](https://user-images.githubusercontent.com/34715963/226595465-cce22f63-0573-4ad2-bedf-e9b64ce8e1e0.png) `onet.pl` ![image](https://user-images.githubusercontent.com/34715963/226595610-633124ac-5847-472b-82c5-1ecbe760a217.png) ![image](https://user-images.githubusercontent.com/34715963/226595645-fb7fe45c-8aa7-41f1-9f7f-956d0773a586.png) `brave://settings/shields/filters` ![image](https://user-images.githubusercontent.com/34715963/226596419-d8bc9b7b-fb85-4d2e-9781-8859a98ab88e.png)

Verified test plan from https://github.com/brave/brave-browser/issues/16863#issuecomment-896457715

launched JS script ![image](https://user-images.githubusercontent.com/34715963/226599766-6e00de02-ca86-4433-9258-c910518a89d2.png) `brave://settings/shields/filters` - verified that the filter is limited to origin domain ![image](https://user-images.githubusercontent.com/34715963/226600306-125edfd5-b0be-427f-8db3-9aa7d4fbba8c.png)
MadhaviSeelam commented 1 year ago

Verification PASSED using

Brave | 1.50.108 Chromium: 112.0.5615.39 (Official Build) (64-bit)
-- | --
Revision | a0e7b9718a92bcd1cf33b7c95316caff3fc20714-refs/branch-heads/5615@{#753}
OS | Windows 11 Version 22H2 (Build 22621.1413)

Steps:

  1. installed 1.50.91
  2. launched Brave
  3. loaded nbcnews.com, time.com, and theguardian.com/us
  4. on each, context-clicked on their logo and chose "Brave-->Block element`
  5. selected the logo/text
  6. clicked on Create
  7. checked the filters in brave://settings/shields/filters
  8. clicked Reload this page, for each site
  9. ensured logos were still blocked
nbcnews.com time.com theguardian.com brave://settings/shields/filters sites without logos
image image image image image