diff --git a/browser/components/newtab/PlacesProvider.jsm b/browser/components/newtab/PlacesProvider.jsm index 31d02d824a55..6b9ebd706e4d 100644 --- a/browser/components/newtab/PlacesProvider.jsm +++ b/browser/components/newtab/PlacesProvider.jsm @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -/* global XPCOMUtils, Services, PlacesUtils, gPrincipal, EventEmitter */ +/* global XPCOMUtils, Services, PlacesUtils, EventEmitter */ /* global gLinks */ /* exported PlacesProvider */ @@ -22,52 +22,14 @@ XPCOMUtils.defineLazyGetter(this, "EventEmitter", function() { return EventEmitter; }); -XPCOMUtils.defineLazyGetter(this, "gPrincipal", function() { - let uri = Services.io.newURI("about:newtab"); - return Services.scriptSecurityManager.getNoAppCodebasePrincipal(uri); -}); - XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils", + "resource://gre/modules/NewTabUtils.jsm"); // The maximum number of results PlacesProvider retrieves from history. const HISTORY_RESULTS_LIMIT = 100; -/** - * Singleton that checks if a given link should be displayed on about:newtab - * or if we should rather not do it for security reasons. URIs that inherit - * their caller's principal will be filtered. - */ -let LinkChecker = { - _cache: new Map(), - - get flags() { - return Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL | - Ci.nsIScriptSecurityManager.DONT_REPORT_ERRORS; - }, - - checkLoadURI: function LinkChecker_checkLoadURI(aURI) { - if (!this._cache.has(aURI)) { - this._cache.set(aURI, this._doCheckLoadURI(aURI)); - } - - return this._cache.get(aURI); - }, - - _doCheckLoadURI: function LinkChecker_doCheckLoadURI(aURI) { - let result = false; - try { - Services.scriptSecurityManager. - checkLoadURIStrWithPrincipal(gPrincipal, aURI, this.flags); - result = true; - } catch (e) { - // We got a weird URI or one that would inherit the caller's principal. - Cu.reportError(e); - } - return result; - } -}; - /* Queries history to retrieve the most visited sites. Emits events when the * history changes. * Implements the EventEmitter interface. @@ -105,7 +67,8 @@ Links.prototype = { aNewFrecency, aGUID, aHidden, aLastVisitDate) { // jshint ignore:line // The implementation of the query in getLinks excludes hidden and // unvisited pages, so it's important to exclude them here, too. - if (!aHidden && aLastVisitDate) { + if (!aHidden && aLastVisitDate && + NewTabUtils.linkChecker.checkLoadURI(aURI.spec)) { gLinks.emit("linkChanged", { url: aURI.spec, frecency: aNewFrecency, @@ -122,10 +85,12 @@ Links.prototype = { }, onTitleChanged: function historyObserver_onTitleChanged(aURI, aNewTitle) { - gLinks.emit("linkChanged", { - url: aURI.spec, - title: aNewTitle - }); + if (NewTabUtils.linkChecker.checkLoadURI(aURI.spec)) { + gLinks.emit("linkChanged", { + url: aURI.spec, + title: aNewTitle + }); + } }, QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver, @@ -184,7 +149,7 @@ Links.prototype = { params: {limit: this.maxNumLinks} }); - return links.filter(link => LinkChecker.checkLoadURI(link.url)); + return links.filter(link => NewTabUtils.linkChecker.checkLoadURI(link.url)); }), /** @@ -247,6 +212,10 @@ Links.prototype = { const gLinks = new Links(); // jshint ignore:line let PlacesProvider = { - LinkChecker, links: gLinks, }; + +// Kept only for backwards-compatibility +XPCOMUtils.defineLazyGetter(PlacesProvider, "LinkChecker", + () => NewTabUtils.linkChecker); + diff --git a/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js b/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js index e35a0e4e6455..3494e87222df 100644 --- a/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js +++ b/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js @@ -59,13 +59,13 @@ function makeVisit(index, daysAgo, isTyped, domain = TEST_URL) { add_task(function test_LinkChecker_securityCheck() { let urls = [ - {url: "file://home/file/image.png", expected: false}, - {url: "resource:///modules/PlacesProvider.jsm", expected: false}, {url: "javascript:alert('hello')", expected: false}, // jshint ignore:line {url: "data:image/png;base64,XXX", expected: false}, {url: "about:newtab", expected: true}, {url: "https://example.com", expected: true}, {url: "ftp://example.com", expected: true}, + {url: "file://home/file/image.png", expected: true}, + {url: "resource:///modules/PlacesProvider.jsm", expected: true}, ]; for (let {url, expected} of urls) { let observed = PlacesProvider.LinkChecker.checkLoadURI(url); diff --git a/toolkit/modules/NewTabUtils.jsm b/toolkit/modules/NewTabUtils.jsm index bed620be7e4e..e5dd5e950836 100644 --- a/toolkit/modules/NewTabUtils.jsm +++ b/toolkit/modules/NewTabUtils.jsm @@ -23,11 +23,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "PageThumbs", XPCOMUtils.defineLazyModuleGetter(this, "BinarySearch", "resource://gre/modules/BinarySearch.jsm"); -XPCOMUtils.defineLazyGetter(this, "gPrincipal", function() { - let uri = Services.io.newURI("about:newtab"); - return Services.scriptSecurityManager.createCodebasePrincipal(uri, {}); -}); - XPCOMUtils.defineLazyGetter(this, "gCryptoHash", function() { return Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash); }); @@ -1351,8 +1346,12 @@ var LinkChecker = { _doCheckLoadURI: function Links_doCheckLoadURI(aURI) { try { + // about:newtab is currently privileged. In any case, it should be + // possible for tiles to point to pretty much everything - but not + // to stuff that inherits the system principal, so we check: + let systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal(); Services.scriptSecurityManager. - checkLoadURIStrWithPrincipal(gPrincipal, aURI, this.flags); + checkLoadURIStrWithPrincipal(systemPrincipal, aURI, this.flags); return true; } catch (e) { // We got a weird URI or one that would inherit the caller's principal.