brave / brave-browser

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

News' "add this RSS feed" functionality doesn't honor the HTTPS upgrade setting #38282

Open fmarier opened 6 months ago

fmarier commented 6 months ago

Steps To Reproduce:

  1. Enable Brave News
  2. In Brave settings (brave://settings/shields), set "Upgrade connections to HTTPS" to "Strict"
  3. Open WireShark. Enable capturing. Set the display filter in WireShark to http
  4. Visit https://fmarier.github.io/brave-testing/news-add-rss.html
  5. Click the feed icon in the URL bar
  6. Go back to WireShark. You can see a plaintext request sent to 172.105.6.87

Actual

Screenshot from 2024-05-13 10-52-56

Expected

The request should be upgraded to HTTPS and no HTTP request should be visible in WireShark.

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

fmarier commented 6 months ago

The other thing that this suggests is that we are likely not running these URLs through our privacy filters (e.g. debouncer, query string filter).

fmarier commented 6 months ago

To confirm, I updated the test page to add an fbclid parameter to the URL and it doesn't get stripped out: Screenshot from 2024-05-13 12-15-18

bsclifton commented 6 months ago

cc: @LorenzoMinto

diracdeltas commented 5 months ago

@bsclifton is anyone able to take this issue?

diracdeltas commented 2 months ago

@boocmp do you think you could take this? assuming @LorenzoMinto / @fallaciousreasoning have not started on it.

fallaciousreasoning commented 2 months ago

@fmarier I suspect this is probably a broader problem with our ApiRequestHelper class so this probably affects everything that uses it :fearful:

boocmp commented 2 months ago

ApiRequestHelper uses the SharedUploaderFactory which bypasses any Brave code for requests passing through it, I always thought it is by design.

fallaciousreasoning commented 2 months ago

cc @petemill not really sure on the best approach here if its by design that the APIRequestHelper isn't going through Brave's request handling code?

fmarier commented 2 months ago

I think this should be a News-specific fix (for these custom RSS feeds). We should not change any other internal requests.