Skip to content

GitLab

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
Trac
Trac
  • Project overview
    • Project overview
    • Details
    • Activity
  • Issues 246
    • Issues 246
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Operations
    • Operations
    • Metrics
    • Incidents
  • Analytics
    • Analytics
    • Value Stream
  • Wiki
    • Wiki
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Create a new issue
  • Issue Boards

GitLab is used only for code review, issue tracking and project management. Canonical locations for source code are still https://gitweb.torproject.org/ https://git.torproject.org/ and git-rw.torproject.org.

  • Legacy
  • TracTrac
  • Issues
  • #34383

Closed (moved)
Open
Opened Jun 05, 2020 by Georg Koppen@gk

Accept request if GetHost() in mixed content blocking check fails

While reviewing acat's rebase work in #33533 (moved) I noticed the following block in our patch for #23247 (moved):

   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 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(). 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.

To upload designs, you'll need to enable LFS and have admin enable hashed storage. More information
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
Reference: legacy/trac#34383