about:tor should not have chrome privileges

We should ensure that about:tor runs with only content privileges.

Changing the getURIFlags() function in src/components/aboutTor.js to include Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT in the value returned should do the trick, but other things will need to be fixed as a result of that change.

added TorBrowserTeam201706R component::applications/tor browser ff52-esr owner::mcs priority::medium resolution::fixed severity::normal status::closed type::defect labels

Trac:
Cc: brade to brade, gk

Does this also need to be done for the changelog about page popup or is it the same page as about:tor?

Replying to cypherpunks:

Does this also need to be done for the changelog about page popup or is it the same page as about:tor?

It is a separate page (about:tbupdate) but it already runs with only content privileges.

Actual for e10s.

Trac:
Keywords: N/A deleted, ff52-esr added
Component: Applications/Torbutton to Applications/Tor Browser

Replying to cypherpunks:

Actual for e10s.

Do you mean that with e10s enabled, we must avoid requiring chrome privileges? I do not think that is true, so maybe you mean something else?

Trac:
Keywords: N/A deleted, TorBrowserTeam201706 added

Here is a patch for review: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-01&id=984af558af58bb8715af72c4811acc7fc4253bc1 This change fixes #21948 (moved) and #22525 (moved) as well, so it would be great to include it in a Tor Browser release soon. While the patch is somewhat large, that is mainly because we had to move a lot of code out of torbutton.js into the new aboutTor-content.js content script (so it can run in the content process where the about:tor DOM is accessible).

Trac:
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R added
Status: new to needs_review

I forgot to mention that Kathy and I did some testing of this patch on OSX, Linux64, and Windows 10.

Replying to mcs:

Here is a patch for review: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-01&id=984af558af58bb8715af72c4811acc7fc4253bc1 This change fixes #21948 (moved) and #22525 (moved) as well, so it would be great to include it in a Tor Browser release soon. While the patch is somewhat large, that is mainly because we had to move a lot of code out of torbutton.js into the new aboutTor-content.js content script (so it can run in the content process where the about:tor DOM is accessible).

Looks good to me, thanks! Just some nits:

+  // process that is only available here (in the chrome process). It is sent
+  // sent to the content process when an about:tor window is opened and in

just one "sent"

+   kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ],
+
+   get isAboutTor() {
+    return content.document.documentURI.toLowerCase() == "about:tor";

Indentation

"the Tor Button item's x coordinate" -> "the x coordinate of Torbutton's toolbar item"

"torbutton toolbar item" -> "Torbutton toolbar item"

Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201706R deleted, TorBrowserTeam201706 added

Replying to gk:

Looks good to me, thanks! Just some nits: ...

Thanks for your review! Here is a revised patch that fixes the things you found: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-02

Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R added

Sorry, I should have given more context. You have now

+var AboutTorListener = {
+   kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ],
+
+   get isAboutTor() {
+     return content.document.documentURI.toLowerCase() == "about:tor";
+  },

but I'd like to have it like

+var AboutTorListener = {
+  kAboutTorMessages: [ "AboutTor:ChromeData", "AboutTor:ToolbarData" ],
+
+  get isAboutTor() {
+    return content.document.documentURI.toLowerCase() == "about:tor";
+  },

Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201706R deleted, TorBrowserTeam201706 added

I am sorry we botched the indentation fix, and I am sorry that we then forgot to revise the patch for the past two days. Do you have a git tool to spot those kind of errors, or just "eagle eyes?

In any case, here is a revised patch: https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug18913-03

Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam201706 deleted, TorBrowserTeam201706R added

Just "eagle eyes". :) Applied to master (commit 32f9cf56c89ccd2e24924975dc5515e4198d28c3).

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue #22535 (moved)

mentioned in issue #22884 (moved)

mentioned in issue #24421 (moved)

moved to tpo/applications/tor-browser#18913 (closed)

mentioned in issue tpo/applications/tor-browser#22884 (closed)