#23723 closed defect (fixed)

Loading entities from NoScript .dtd files is blocked

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: noscript
Cc: ma1, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Starting with NoScript 5.1.0 there is the following error in the browser console visible:

XML Parsing Error: undefined entity
Location: jar:file:///home/thomas/Arbeit/Tor/tor-browser-bundle/tor-browser_en-US/Browser/TorBrowser/Data/Browser/profile.default/extensions/%7B73a6fe31-595d-460b-a920-fcc0f8843232%7D.xpi!/chrome/content/noscript/noscriptOverlayFx57.xul?1brcl6s241gsnehuud9g
Line Number 27, Column 5:  noscriptOverlayFx57.xul:27:5

NoScript is retrieving its XUL overlay template through XMLHttpRequest which seems to break due to failing entity resolution.

Child Tickets

Change History (10)

comment:1 Changed 13 months ago by gk

comment:19:ticket:23718 seems to indicate that our defense against fingerprinting users by probing for resource://- and chrome://-URIs might be the culprit: a workaround would be to set extensions.torbutton.resource_and_chrome_uri_fingerprinting to true.

comment:2 Changed 13 months ago by gk

So, it still seems to me that this issue alone is *not* breaking NoScript funtionality. Or am I missing something?

comment:3 Changed 13 months ago by cypherpunks

Something with whitelist, but could be unrelated.
As our #8725 is more restrictive than Mozilla's implementation (landed on FF57), what is the right direction to go: enforce & upstream (and fix NoScript) or relax & backport?

comment:4 in reply to:  1 Changed 13 months ago by ma1

Replying to gk:

: a workaround would be to set extensions.torbutton.resource_and_chrome_uri_fingerprinting to true.

I'm implementing this in next dev build, out in minutes, and it apparently works, with exposure minimized to the quasy-synchronous (local filesystem) load/parsing.

You can preview it by running this code in a browser-scoped Scratchpad window with any NoScript 5.1.x installed:

{
  let xhr = new XMLHttpRequest();
  xhr.open("GET", "chrome://noscript/content/noscriptOverlayFx57.xul");
  try {
    // work around to resolve overlay's XML entities despite the Tor Browser
    
    let TOR_PREF = "extensions.torbutton.resource_and_chrome_uri_fingerprinting";
    let torPrefValue = Services.prefs.getBoolPref(TOR_PREF);
    let restorePref = () => Services.prefs.setBoolPref(TOR_PREF, torPrefValue);
    for (let e of ["progress", "loadend"]) { // restore as early as possible (almost sync)
      xhr.addEventListener(e, restorePref);
    }
    xhr.addEventListener("loadstart", () => {
      Services.prefs.setBoolPref(TOR_PREF, true);
    });
    
  } catch (e) {
    // no pref value, it doesn't seem to be a Tor Browser :)
  }
  xhr.addEventListener("load", () => {
    alert(xhr.responseXML.getElementById("noscript-tbb"));
  });
 xhr.send()
}

Last edited 13 months ago by ma1 (previous) (diff)

comment:5 Changed 13 months ago by ma1

Notice that 5.1.2rc1 with the work-around mentioned above is now out on AMO (Beta channel) and https://noscript.net/getit#devel

Please let me know if it needs to go in the release channel or a different approach is preferred, thanks.

comment:6 in reply to:  5 Changed 13 months ago by gk

Replying to ma1:

Notice that 5.1.2rc1 with the work-around mentioned above is now out on AMO (Beta channel) and https://noscript.net/getit#devel

Please let me know if it needs to go in the release channel or a different approach is preferred, thanks.

Thanks. I think we should wait a bit until it hits your release channel until we figured out what is causing #23724. Could you have a look at that one, too? It seems to me there are things broken in vanilla Firefox 52 ESR as well when the update happens. I am not sure yet whether it is a bad as in Tor Browser but there are similar error messages like

CustomizableUI:Widget 'noscript-tbb' not found, unable to move  CustomizableUI.jsm:1149
can't access dead object  WebExt.js:17
	tell chrome://noscript/content/WebExt.js:17:7
	saveData chrome://noscript/content/WebExt.js:7:5
	ns.savePrefs chrome://noscript/content/Main.js:1279:9
	ns.dispose chrome://noscript/content/Main.js:891:7
	ns.shutdown chrome://noscript/content/Main.js:216:5
	autoSync/obj[method] chrome://noscript/content/e10sIPC.js:45:16
	shutdown chrome://noscript/content/Restartless.jsm:58:3
	shutdown jar:file:///home/thomas/.mozilla/firefox/bm8w2fpw.noscript1/extensions/%7B73a6fe31-595d-460b-a920-fcc0f8843232%7D.xpi!/bootstrap.js:30:5
	this.XPIProvider.callBootstrapMethod resource://gre/modules/addons/XPIProvider.jsm:4968:9
	startInstall/< resource://gre/modules/addons/XPIProvider.jsm:5859:15
	InterpretGeneratorResume self-hosted:1213:8
	next self-hosted:1120:9
	TaskImpl_run resource://gre/modules/Task.jsm:319:42
	bound TaskImpl_run self-hosted:957:17
	Handler.prototype.process resource://gre/modules/Promise-backend.js:932:23
	this.PromiseWalker.walkerLoop resource://gre/modules/Promise-backend.js:813:7
	bound  self-hosted:913:17
	bound bound  self-hosted:913:17
	this.PromiseWalker.scheduleWalkerLoop/< resource://gre/modules/Promise-backend.js:747:11
No chrome package registered for chrome://noscript/locale/about.properties

that make me nervous. For instance one visible thing is that the NoScript icon gets suddenly moved to the right side of the toolbar, a thing we can observe in Tor Browser as well.

comment:7 Changed 13 months ago by cypherpunks

Status: newneeds_information

??? How did you test that?
For correct testing it's necessary to do a restartless update of the 5.1.2rc1 (with workaround) to something newer.
Update: Oh, wait. You're testing another bug...

Last edited 13 months ago by cypherpunks (previous) (diff)

comment:8 in reply to:  7 Changed 13 months ago by gk

Status: needs_informationassigned

Replying to cypherpunks:

??? How did you test that?
For correct testing it's necessary to do a restartless update of the 5.1.2rc1 (with workaround) to something newer.
Update: Oh, wait. You're testing another bug...

Trying to figure out whether entities in .dtd files get properly resolved does not necessarily need to get tested via an update I think. I guess you mean #23724.

comment:9 Changed 13 months ago by arthuredelstein

Cc: arthuredelstein added

comment:10 Changed 12 months ago by gk

Resolution: fixed
Status: assignedclosed

Thanks for fixing, Giorgio.

Note: See TracTickets for help on using tickets.