Opened 4 months ago

Last modified 5 days ago

#29646 new defect

NoScript XSS user choices are persisted

Reported by: atac Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-disk-leak xss noscript tbb-newnym ux-team
Cc: ma1, antonela Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Whenever user chooses 'Always allow' or 'Always block' in one of the NoScript XSS popups the setting is persisted in storage-sync.sqlite file and this is never cleared on browser startup as the rest of NoScript preferences.

The full persisted object can be inspected via about:debugging -> Debug Noscript -> browser.storage.sync.get('xssUserChoices').

I understand this is not intended behaviour, since NoScript default is to not persist user choices (clearing them up on browser start).

Child Tickets

Change History (7)

comment:1 Changed 4 months ago by gk

Keywords: noscriptm tbb-newnym added; noscript removed

One could actually argue that it's exactly behaving as expected: You said *always*, now you get always (while just simply allowing/blocking would be session-wide (Or maybe it's bound to the domain? I have not checked)).

That persists over New Identity, which is definitely a bug. But I am not sure what the best solution for the disk persistence would be. Just not offering those two options on the dialog? Or maybe we should just disable NoScript's XSS protections altogether given that it causes bugs like #29647 and #22362?

comment:2 Changed 8 days ago by arma

I think 'always' needs to mean 'for this session, because of course we don't remember anything about your browsing activity across sessions'.

And if we want to get better at the interface, we should make sure that we never offer something that implies always, and instead it's always phrased as "for the rest of this session".

comment:3 Changed 8 days ago by cypherpunks

That persists over New Identity, which is definitely a bug.

Another bug for another ticket.

But I am not sure what the best solution for the disk persistence would be.

Disk Avoidance from the design guide. Settings should be in-memory only.

Keywords: noscriptm?
Does ma1 know about it?

comment:4 Changed 8 days ago by gk

Cc: ma1 added
Keywords: noscript added; noscriptm removed

comment:5 Changed 6 days ago by ma1

I could add a checkbox "[ ] Forget my choice at the end of this session" which would be pre-checked for incognito tabs and in the Tor Browser (keeping this setting just in memory, without persistence), unchecked otherwise.
Would this work?

comment:6 in reply to:  5 Changed 5 days ago by gk

Cc: antonela added
Keywords: ux-team added

Replying to ma1:

I could add a checkbox "[ ] Forget my choice at the end of this session" which would be pre-checked for incognito tabs and in the Tor Browser (keeping this setting just in memory, without persistence), unchecked otherwise.
Would this work?

This sounds like an improvement, thanks! I am not convinced yet that we should have this option checked by default, though. Here is my current thinking:

13:34 <+GeKo> my current thinking is that we should try to stick to two choices
13:34 <+GeKo> and the default selected one is the per-site exception as it is right 
              now
13:35 <+GeKo> and then the second option would bewhat ma1 suggest if we are in 
              privtae browsing mode
13:35 <+GeKo> or if we are not in that mode we'd get what we currently have
13:35 <+GeKo> i wonder whether the noscript ui could be that flexible, though
13:36 <+GeKo> the risk with ma1's proposal is that we make it easy to fingerprint 
              users
13:36 <+GeKo> sure
13:37 <+GeKo> because as soon as you have (say, by accident) whitelist some xss 
              request
13:38 <+GeKo> for the session it is detectable by any other website by embedding 
              similar requests

Adding antonela for UX input.

comment:7 Changed 5 days ago by antonela

The best approach is the one which balance XSS warnings and usability. I have concerns about how users interact with the XSS warning screen. So, having another option available there will not solve this problem for the masses nor allow users to pick the safest option for them.

That said if Tor Browser can keep that option across sessions, it will improve the overall experience for recurrent users visiting a website recurrently. Let's say I'm a user visiting foo.com and I got an XSS warning, I'm blocking requests because I want to be safe and I continue browsing in a half-loaded website. Maybe I can deal with that brokerage but be safe enough. That is the current Tor Browser users experience so far.

As a damage reduction, having the option persistent per-session (block or allow) seems the best balance between risk and usability. If a user wants a website loading correctly (or choose to allow, say by accident :), and we have concerns about leaking, that will happen just in the current session.

You may argue that this is not strictly related to security, but on users end it is. Maybe, it fits on something to consider for our security settings, where we should holistically balance security and usability across levels.

Note: See TracTickets for help on using tickets.