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

Limit URL requests to whitelisted schemes in windows with Tor #5357

Closed
riastradh-brave opened this issue Jul 23, 2019 · 2 comments · Fixed by brave/brave-core#2993
Closed
Assignees
Labels
bug feature/tor/leakproofing Eliminating unexpected ways that someone using Tor might be unmasked. feature/tor QA/Test-Plan-Specified QA/Yes regression

Comments

@riastradh-brave
Copy link
Contributor

riastradh-brave commented Jul 23, 2019

In brave/brave-core#2647, to fix #4312, we lost some logic to block URL requests outside a small whitelist that are safe through Tor or don't need Tor:

  if (request_url.SchemeIsHTTPOrHTTPS()) {
    auto* proxy_service = ctx->request->context()->proxy_resolution_service();
    return tor_profile_service->SetProxy(proxy_service, request_url, false);
  } else if (request_url.SchemeIs(content::kChromeUIScheme) ||
             request_url.SchemeIs(extensions::kExtensionScheme) ||
             request_url.SchemeIs(content::kChromeDevToolsScheme)) {
    // No proxy for internal schemes.
    return net::OK;
  } else {
    return net::ERR_DISALLOWED_URL_SCHEME;
  }

brave/brave-core@d7db72e#diff-de4449937f0f4de57a0104b7b425bafbL47-L57

We should find a place to restore that so that we do not inadvertently allow URL requests with schemes that are unsafe through Tor. (It is conceivable that all URL schemes are safe, but this seems unlikely and requires a much more substantial audit than we have conducted in order to inspire confidence.)

@riastradh-brave riastradh-brave added bug feature/tor/leakproofing Eliminating unexpected ways that someone using Tor might be unmasked. regression labels Jul 23, 2019
@diracdeltas
Copy link
Member

is this just a dupe of #4461 ?

@riastradh-brave
Copy link
Contributor Author

For posterity: it is not quite a duplicate because #4461 is about external protocol handlers, while this is about internally handled protocols. But Anthony's change should fix both in one swell foop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature/tor/leakproofing Eliminating unexpected ways that someone using Tor might be unmasked. feature/tor QA/Test-Plan-Specified QA/Yes regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants