[Desktop] [hackerone] [Security] webtorrent should clear redirect URL if not http:// or https://

[diracdeltas]

Test plan

See brave/brave-core#5595 (comment)

Description

originally reported at https://hackerone.com/reports/847848

this is a very minor phishing risk.

STR:

1. make sure webtorrent is enabled (by accessing https://webtorrent.io/torrents/sintel.torrent)
2. enter chrome-extension://lgjmpdmojkpocjcopdikifhejkkjglho/extension/brave_webtorrent.html?chrome://wallet/ in the urlbar and hit enter
3. urlbar shows brave://wallet with a blank page
4. click urlbar and hit enter again. wallet loads.

Expected behavior:
In step 3, either nothing should happen or you should be navigated to about:blank

(Expected behavior updated by @yrliou ) In step 3, a blank extension page is shown with original URL (chrome-extension://lgjmpdmojkpocjcopdikifhejkkjglho/extension/brave_webtorrent.html?chrome://wallet/) and nothing will be changed when doing step 4.

[fmarier]

Here are the steps I found to reproduce this reliably:

1. Start the browser.
2. Go into chrome://settings.
3. Toggle WebTorrent OFF, and then ON.
4. Load the test page at https://fmarier.github.io/brave-testing/brave-browser-9159.html.
5. Click on one of the last two links.

If I skip Step 3, I get the following error page:

This suggests that perhaps the extension load order matters.

[fmarier]

Based on the Slack thread, I made this simple fix:

--- a/components/brave_webtorrent/browser/net/brave_torrent_redirect_network_delegate_helper.cc
+++ b/components/brave_webtorrent/browser/net/brave_torrent_redirect_network_delegate_helper.cc
@@ -54,13 +54,16 @@ int OnHeadersReceived_TorrentRedirectWork(
 
   if (!original_response_headers ||
       !IsMainFrameResource(ctx) ||
+      !ctx->request_url.SchemeIsHTTPOrHTTPS() ||
       ctx->is_webtorrent_disabled ||
       // download .torrent, do not redirect
       (IsWebtorrentInitiated(ctx) && !IsViewerURL(ctx->request_url)) ||
       !IsTorrentFile(ctx->request_url, original_response_headers)) {
+    LOG(ERROR) << "NO REDIRECT";
     return net::OK;
   }
 
+  LOG(ERROR) << "REDIRECT";
   *override_response_headers =
     new net::HttpResponseHeaders(original_response_headers->raw_headers());
   (*override_response_headers)

but it didn't help since it's not even entering that function and printing anything to the console when I click on the offending links.

[fmarier]

@feross Was that the fix you were trying to test or did you have something else in mind?

[feross]

This suggests that perhaps the extension load order matters.

I think this is just a quirk of how the lazy-loading feature was implemented. The WebTorrent extension is not loaded until it's needed. So it seems like linking directly to an extension URL doesn't cause the extension to get enabled. Going into settings and toggling it OFF -> ON seems to force it to be enabled. Another option to your steps 1-3 is to load a torrent from https://webtorrent.io/free-torrents which has the same effect.

[feross]

@feross Was that the fix you were trying to test or did you have something else in mind?

Indeed, this was the intended fix. Are neither of your logs ("NO REDIRECT" and "REDIRECT") getting printed out?

I think the issue may actually be in brave/components/brave_webtorrent/browser/content_browser_client_helper.h in the TranslateTorrentUIURLReversed method which handles the URL rewriting.

https://github.com/brave/brave-core/blob/2ccf47f6ecdc6694a10b003dd8ba520d8bd42546/components/brave_webtorrent/browser/content_browser_client_helper.h#L42-L49

If I understand that code correctly, it appears to be translating the URL by just taking the unescaped query portion of the original URL, without first checking that it is HTTP or HTTPS.

So the URL chrome-extension://lgjmpdmojkpocjcopdikifhejkkjglho/extension/brave_webtorrent.html?chrome://settings/ gets translated to chrome://settings/

@fmarier Can you try moving the check into that function instead?

[yrliou]

Test plan specified in brave/brave-core#5595 (comment)

[LaurenWags]

Verified passed with

Brave | 1.11.73 Chromium: 83.0.4103.106 (Official Build) dev (64-bit)
-- | --
Revision | ce7134bb3d95141cd18f1e65772a4247f282d950-refs/branch-heads/4103@{#694}
OS | macOS Version 10.14.6 (Build 18G3020)

• Verified test plan from Do not reverse rewrite the webtorrent URL when query string is not HT… brave-core#5595

Step 3:

Step 5:

---

Verification passed on

Brave | 1.11.78 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
-- | --
Revision | 8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS | Windows 10 OS Version 1903 (Build 18362.900)

• Verified test plan from Do not reverse rewrite the webtorrent URL when query string is not HT… brave-core#5595

Step 3:

Step 5:

---

Verified passed with

Brave	1.11.80 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
Revision	8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS	Linux

• Verified test plan from Do not reverse rewrite the webtorrent URL when query string is not HT… brave-core#5595

Step 3:

Step 5: