[tor-bugs] #34383 [Applications/Tor Browser]: Accept request if GetHost() in mixed content blocking check fails

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#34383: Accept request if GetHost() in mixed content blocking check fails
------------------------------------------+--------------------------------
     Reporter:  gk                        |      Owner:  tbb-team
         Type:  defect                    |     Status:  new
     Priority:  Medium                    |  Milestone:
    Component:  Applications/Tor Browser  |    Version:
     Severity:  Normal                    |   Keywords:
                                          |  TorBrowserTeam202006
Actual Points:                            |  Parent ID:
       Points:                            |   Reviewer:
      Sponsor:                            |
------------------------------------------+--------------------------------
 While reviewing acat's rebase work in #33533 I noticed the following block
 in our patch for #23247:
 {{{
    if (!parentIsHttps) {
 +    nsAutoCString parentHost;
 +    rv = innerRequestingLocation->GetHost(parentHost);
 +    if (NS_FAILED(rv)) {
 +      NS_ERROR("requestingLocation->GetHost failed");
 +      *aDecision = REJECT_REQUEST;
 +      return NS_OK;
 +    }
 +
 +    bool parentIsOnion =
 +        StringEndsWith(parentHost, NS_LITERAL_CSTRING(".onion"));
 +    if (!parentIsOnion) {
 +      *aDecision = ACCEPT;
 +      return NS_OK;
 +    }
 +  }
 }}}
 The corresponding part in Mozilla's code looks like [https://searchfox.org
 /mozilla-esr68/source/dom/security/nsMixedContentBlocker.cpp#748 this]:
 {{{
   if (!parentIsHttps) {
     *aDecision = ACCEPT;
     return NS_OK;
   }
 }}}
 Mozilla does not even check the host but is accepting requests at this
 point outright while we reject them if `GetHost()` fails.

 I wonder whether we should follow them here because I am a little worried
 that we might introduce a subtle bug by diverging from them.

 I guess the question is whether it can be the case that we have a .onion
 URL but the `GetHost()` call is failing. Mozilla does not care as there is
 no decision to be made depending on the *host* but the scheme alone in
 this check. However, we do care.

 Let's look at that from a different angle: the code block we added is
 essentially a check for a .onion domain: if we don't have one, accept the
 request otherwise pass it on for further checks. Let's look what we do
 elsewhere when checking for a .onion domain, e.g. in
 `IsPotentiallyTrustworthyOnion()`. [https://searchfox.org/mozilla-
 esr68/source/dom/security/nsMixedContentBlocker.cpp#401 There] we have:
 {{{
  nsAutoCString host;
   nsresult rv = aURL->GetHost(host);
   NS_ENSURE_SUCCESS(rv, false);
   return StringEndsWith(host, NS_LITERAL_CSTRING(".onion"));
 }}}
 This means if the `GetHost()` check fails we say "no, that's not a
 .onion". Following that logic we would get to `parentIsOnion = false` in
 our code block in question and we should `ACCEPT` the request as well in
 case the `GetHost()` call fails.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/34383>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs