[tor-commits] [torbutton/master] Bug 8725: Consistently deny redirects to browser/addon internal URLs.

[mikeperry at torproject.org]

commit fa67687df5fc72c0b6085d9941331277d32319f3
Author: Yawning Angel <yawning at schwanenlied.me>
Date:   Sat Jul 16 03:37:27 2016 +0000

    Bug 8725: Consistently deny redirects to browser/addon internal URLs.
    
    The browser's behavior is different depending on if a given internal
    resource is available or not, regardless of the fact that the actual
    body will not load due to the various safeguards and checks.
    
    This normalizes the behavior by denying all redirects destined for URLs
    with proscribed browser internal schemes (`resource`, `about`, `chrome`).
---
 src/components/content-policy.js | 41 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/components/content-policy.js b/src/components/content-policy.js
index c6c8aa9..e025ecd 100644
--- a/src/components/content-policy.js
+++ b/src/components/content-policy.js
@@ -8,7 +8,7 @@
  * https://notabug.org/desktopd/no-resource-uri-leak/src/master/src/resource-filter/content-policy.js
  */
 
-const Ci = Components.interfaces, Cu = Components.utils;
+const Cc = Components.classes, Ci = Components.interfaces, Cu = Components.utils;
 
 // Import XPCOMUtils object.
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -21,8 +21,6 @@ ContentPolicy.prototype = {
   contractID: "@torproject.org/content-policy;1",
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIContentPolicy]),
 
-  _xpcom_categories: [{category: "content-policy"}],
-
   shouldLoad: function(aContentType, aContentLocation, aRequestOrigin, aContext, aMimeTypeGuess, aExtra) {
     // Accept if no content URI or scheme is not a resource/chrome.
     if (!aContentLocation || !(aContentLocation.schemeIs('resource') || aContentLocation.schemeIs('chrome')))
@@ -44,5 +42,42 @@ ContentPolicy.prototype = {
   },
 };
 
+// Install a HTTP response handler to check for redirects to URLs with schemes
+// that should be internal to the browser.  There's various safeguards and
+// checks that cause the body to be unavailable, but the `onLoad()` behavior
+// is inconsistent, which results in leaking information about the specific
+// user agent instance (eg: what addons are installed).
+var requestObserver = {
+  ioService: Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService),
+  observerService: Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService),
+
+  start: function() {
+    this.observerService.addObserver(this, "http-on-examine-response", false);
+  },
+
+  observe: function(aSubject, aTopic, aData) {
+    let aChannel = aSubject.QueryInterface(Ci.nsIHttpChannel);
+    let aStatus = aChannel.responseStatus;
+
+    // If this is a redirect...
+    //
+    // Note: Technically `304 Not Modifed` isn't a redirect, but receiving that
+    // to the proscribed schemes is nonsensical.
+    if (aStatus >= 300 && aStatus < 400) {
+      let location = aChannel.getResponseHeader("Location");
+      let aUri = this.ioService.newURI(location, null, null);
+
+      // And it's redirecting into the browser or addon's internal URLs...
+      if (aUri.schemeIs("resource") || aUri.schemeIs("chrome") || aUri.schemeIs("about")) {
+        // Cancel the request.
+        aSubject.cancel(Components.results.NS_BINDING_ABORTED);
+      }
+    }
+  },
+};
+
 // Firefox >= 4.0 (Old versions are extremely irrelevant).
 var NSGetFactory = XPCOMUtils.generateNSGetFactory([ContentPolicy]);
+
+// Register the request observer to handle redirects.
+requestObserver.start();