Opened 3 years ago

Closed 3 years ago

#16775 closed defect (fixed)

about:preferences is broken with security slider set to "High"

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-usability, tbb-5.0-regression, TorBrowserTeam201509R
Cc: brade, mcs, gk, mikeperry, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

If one sets the security slider to "High" there is no way anymore to configure the browser preferences on the about:preferences page. First mentioned in https://blog.torproject.org/blog/tor-browser-50-released#comment-100830.

Child Tickets

Change History (17)

comment:1 Changed 3 years ago by gk

To workaround it change the security level to a lower one, restart, make the changes and set it back to "high".

comment:2 Changed 3 years ago by mcs

Cc: brade mcs added

comment:3 Changed 3 years ago by cypherpunks

Another workaround: set "browser.preferences.inContent" to false in about:config to restore the old Preferences UI that works well, then use (Tools->)Options in the menu (bar).

Last edited 3 years ago by cypherpunks (previous) (diff)

comment:4 Changed 3 years ago by mcs

The underlying preference value that causes this bug is svg.in-content.enabled = false.

A related ticket: #16607 (maybe a similar solution can be used to solve both problems).

comment:5 Changed 3 years ago by mcs

Cc: gk mikeperry added

Kathy and I have concluded that a whitelisting mechanism is needed. The most straightforward solution is to enable SVG when the URI associated with a document has one of the following schemes:

about: chrome: resource:

Doing so will fix this ticket as well as #16607. The only downside is that chrome: and resource: URIs can be loaded by remote web pages, which means they would be able to trigger execution of SVG code in a limited way. Maybe we should have another ticket to disallow that kind of load, but overall the risk seems acceptable.

Before we proceed with a fix, Kathy and I would like opinions from other people as to whether whitelisting is safe. gk? mikeperry?

comment:6 Changed 3 years ago by mcs

Status: newneeds_information

comment:7 Changed 3 years ago by gk

We only need to allow the about: scheme for this bug, right? If so, this is fine to me. Generally, I am very hesitant to water down our "High Security" mode. We know that things are breaking in this mode and users ought to do the same. I don't feel the usability in this mode is so bad at the moment that we should jump off the cliff allowing chrome: and resource: (too) and see what happens.

comment:8 in reply to:  7 Changed 3 years ago by mcs

Keywords: TorBrowserTeam201508R added
Status: needs_informationneeds_review

Replying to gk:

We only need to allow the about: scheme for this bug, right? If so, this is fine to me. Generally, I am very hesitant to water down our "High Security" mode. We know that things are breaking in this mode and users ought to do the same. I don't feel the usability in this mode is so bad at the moment that we should jump off the cliff allowing chrome: and resource: (too) and see what happens.

OK. Whitelisting based on the top-level page turned out to be somewhat messy (for example, some SVG images are loaded as CSS background images, and therefore we need to extract the top document from the channel's load context). We also added some debug printfs because we found them useful for verifying correct behavior. Here is the patch:

https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16775-01&id=0b4c8b589077d054b60a609871dd023bc4d9c444

comment:9 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

Hm... about:tor is not loading for me on Linux anymore and I can't open the hamburger menu nor the browser console with Ctrl + Shift + J. After managing to get the menubar visible trying to open the browser console gives me

console.error: 
  Message: TypeError: devtools.TargetFactory is undefined
  Stack:
    getTarget@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/webconsole/hudservice.js:219:7
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7
this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37

Looking at my branch I built with your patch is the only one atop of the 5.5 branch. Seems like we need some revision here.

comment:10 Changed 3 years ago by gk

Btw: this happens with the default settings, choosing the "High" security level changes nothing.

comment:11 in reply to:  9 ; Changed 3 years ago by mcs

Replying to gk:

Hm... about:tor is not loading for me on Linux anymore and I can't open the hamburger menu nor the browser console with Ctrl + Shift + J.

Strange. Kathy and I did standalone (non-gitian) builds for both Mac OS and Linux64 this morning and did not encounter the problems you found. I just started a gitian-based build to see if that makes a difference (but that build will take a while to complete even though I am only building for Linux).

comment:12 in reply to:  11 Changed 3 years ago by gk

Status: needs_revisionneeds_review

Replying to mcs:

Replying to gk:

Hm... about:tor is not loading for me on Linux anymore and I can't open the hamburger menu nor the browser console with Ctrl + Shift + J.

Strange. Kathy and I did standalone (non-gitian) builds for both Mac OS and Linux64 this morning and did not encounter the problems you found. I just started a gitian-based build to see if that makes a difference (but that build will take a while to complete even though I am only building for Linux).

It looked again and these problems seemed to be caused by me not having a recent Torbutton in the profile (for whatever reason). Sorry for the noise. Testing it it looks good.

comment:13 Changed 3 years ago by gk

The code looks good to me.

comment:14 in reply to:  13 ; Changed 3 years ago by mcs

Cc: arthuredelstein added

Replying to gk:

The code looks good to me.

Thanks. Since these are C++ changes, another review would be very much appreciated.

comment:15 in reply to:  14 Changed 3 years ago by arthuredelstein

Replying to mcs:

Replying to gk:

The code looks good to me.

Thanks. Since these are C++ changes, another review would be very much appreciated.

It looks good to me as well.

comment:16 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201509R added; TorBrowserTeam201508R removed

Transfer review tickets to Sept.

comment:17 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Fixed on tor-browser-38.2.1esr-5.5-2 (commit d8419037dbb3d36115702cc19de9380c6d9c0f10) and on tor-browser-38.2.1esr-5.0-2 (commit 191176cf015236f75da321b2ca0e8ddcd6881a55), thanks.

Note: See TracTickets for help on using tickets.