Closed Bug 971613 Opened 10 years ago Closed 10 years ago

Require setting a pref before modifying a page with Scratchpad or Console

Categories

(DevTools :: General, enhancement)

Product:
DevTools
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: jruderman, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: sec-want) 

[Bug 971597 tracks the self-xss problem. Bug 664589 contains an alternative proposal in which Facebook could opt into extra warnings shown to Firefox users.]

A) By default, don't allow any scripts:

  1) In Console, allow only things like 'help()' and 'clear()'
  2) In Scratchpad, disable the 'run' button
  3) Show a site-specific checkbox near the disabled button:
    [x] Allow my _facebook.com_ account to be hijacked if I paste malicious scripts

B) By default, allow "core JS language" scripts that don't touch the page:

  1) Make Scratchpad and Console-scripts target a non-page global by default
  2) Copy some things onto this global: 'alert', 'atob', 'dump', 'screen'.
  3) If an entered script touches 'document' or 'window', show the pref
  4) The pref could be site-specific or global

C) By default, allow inspecting the page but not modifying it:

  1) Use X-Ray wrappers to give a pure view of the DOM?
  2) Allow [[get]] but not [[set]] or [[call]]?
  3) Upon attempts to [[set]] or [[call]], show the pref
No longer blocks: dev-self-xss
(In reply to Jesse Ruderman from comment #0)
> B) By default, allow "core JS language" scripts that don't touch the page:
> 
>   1) Make Scratchpad and Console-scripts target a non-page global by default
>   2) Copy some things onto this global: 'alert', 'atob', 'dump', 'screen'.
>   3) If an entered script touches 'document' or 'window', show the pref
>   4) The pref could be site-specific or global

This is where we were before and people complained because the sandbox didn't behave as they expected (global vars, instanceof, etc.). Why would we want to go over this again?
Past, can you point me at the previous bug where that change was made? One thing that has changed is that there are now active attacks against Facebook users using the web console, but it would be interesting to see the original reasoning.
Flags: needinfo?(past)
I think that requiring a pref hurts developers too much and goes way beyond what's needed to protect users.

We don't need to (and we can't) make self-xss impossible, we just need to make it hard enough that scammers can't make a profit out of self-xss via our developer tools.

So istm that bug 664589 is a good proposal, but options like bug 953166 or simply having the default tab be the inspector would very possibly be enough.
(In reply to Joe Walker [:jwalker] from comment #3)
> or simply having the default tab be the inspector would very possibly be enough.

I take that back, we have key sequences for the web console which would skip the default panel.
If the first-time tab were the inspector and the default tab after that was "the last one you used" it might help the naive folks and end up giving heavy users more what they want. Speaking for myself I much more often want the inspector than the command line.
The move from using a sandbox to use the Debugger API happened in bug 783499, but the reasons were many and some unrelated. You can see some of the discussion about problems with the sandbox in bug 665834, bug 641629, bug 688401, bug 774365, bug 774753 and bug 690529. Scratchpad was using the same approach as the web console and its users were equally confused.
Flags: needinfo?(past)
The old "sandbox" was only a difference of scope, intended to keep "var a = 4;" from accidentally polluting the page scope. That's totally different from what I'm proposing.
Joe, why do you think having a pref would hurt developers? Note that I'm suggesting that any attempt to interact with the page would show you at least a link to the pref, if not the pref itself.
(In reply to Jesse Ruderman from comment #7)
> The old "sandbox" was only a difference of scope, intended to keep "var a =
> 4;" from accidentally polluting the page scope. That's totally different
> from what I'm proposing.

Oh, in that case I'm confused as to what is your actually proposing. Can you please provide some more details?

For context, the previous code would create a sandbox with the page global as its prototype and a bunch of helper functions attached to it. Are you perhaps suggesting that the sandbox wouldn't have the window as its prototype, but instead trap any accesses to its properties and then either show the pref or pass the calls we deem safe directly to the window?
(In reply to Jesse Ruderman from comment #8)
> Joe, why do you think having a pref would hurt developers? Note that I'm
> suggesting that any attempt to interact with the page would show you at
> least a link to the pref, if not the pref itself.

I don't this that this is a totally soluble problem without fixing all instances of human stupidity. All we can do is to make the instructions complex enough that it's no longer economically viable. And my hunch is that we don't have to make it that much harder to achieve that. For example, on windows WIN+R is a system wide version that isn't exploited as far as I can see.

In my opinion, setting a pref goes too far for many reasons:

* It's a hassle for developers and we want to make developing for the web easier
* It brings the risk that developers are going to get it wrong and break their browsers
* Or worse that it encourages attack scripts that ask users to mess with about:config
* We already have a profile problem for developers, and this makes this problem worse

This is enough that bug 953166 is a better solution, but for me the real killer is that bug 664589 is this idea done right.
(In reply to Panos Astithas [:past] from comment #9)
> Are you
> perhaps suggesting that the sandbox wouldn't have the window as its
> prototype, but instead trap any accesses to its properties and then either
> show the pref or pass the calls we deem safe directly to the window?

Yes, that's (B) in comment 0.
Perhaps we could achieve this by having a second different-origin "dummy eval window" member object for webconsole.js? Instead of using the usual getters and setters to obtain the window object, it simply creates one if necessary and continues to use it.

That way, the code would get injected[1] into this window instead. All the JS APIs will still work, but you won't have much DOM to play with.


 [1]: http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/webconsole.js#925
Huh? It will totally defeat the original purpose of the console.
(In reply to Masatoshi Kimura [:emk] from comment #13)
> Huh? It will totally defeat the original purpose of the console.

The behavior can be disabled. This is to stop users from shooting themselves in the foot by copying arbitrary stuff to the console.

I prefer the way mentioned in Joe's post[1], but the internal implementation can be like this to allow for some usage of javascript.


 [1]: http://incompleteness.me/blog/2011/12/14/combating-self-xss/
I'm making this as a duplicate of bug 664589 for a number of reasons:

* I'm not aware of any serious objection to bug 664589 (using CSP to prevent JS execution with opt-out) so it looks better than this solution
* We'd prefer focus on bug 664589 rather than this bug.

It's worth pointing out that we're also planning a 'landing page' in bug 953166 which should help this problem.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
This is not a dup of bug 664589.

I am not a fan of only fixing this security problem for sites that ask for special protection (bug 664589). If adopted selectively, it will only protect sites where attacks are wormable, and not against the more worrisome targeted attacks against more sensitive sites. If adopted widely, it's an unscalable waste of effort and bandwidth.

A general introduction to dev tools (bug 953166) will help, but since it covers all of dev tools rather than just the most dangerous pieces (UI for injecting scripts into web pages), I don't expect it to be sufficient.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
What's special about dev tools that you think we should only fix the security problem for a small number of sites, and leave it open everywhere else? We generally only do that when it's the only way to overcome severe compatibility problems.
(In reply to Joe Walker [:jwalker] from comment #10)

> I don't this that this is a totally soluble problem without fixing all
> instances of human stupidity.

Calling victims stupid does not solve anything.

> All we can do is to make the instructions
> complex enough that it's no longer economically viable.

This isn't about adding complexity. This is about ensuring the instructions take users past something that (1) is clearly part of Firefox rather than the site (2) explains the risk rather than just being a bunch of UI users don't understand.

> And my hunch is that
> we don't have to make it that much harder to achieve that. For example, on
> windows WIN+R is a system wide version that isn't exploited as far as I can
> see.

https://en.wikipedia.org/wiki/Wikipedia:Other_stuff_exists applies here. *My* hunch is that attackers *will* move to Win+R, and Microsoft will eventually respond.
(In reply to Jesse Ruderman from comment #18)
> https://en.wikipedia.org/wiki/Wikipedia:Other_stuff_exists applies here.
> *My* hunch is that attackers *will* move to Win+R, and Microsoft will
> eventually respond.

Attackers sure are taking their time to move to Win+R because that shortcut has been present for more than 20 yrs now.

The fact that the shortcut (rather the feature) has led to another fact that almost all the users are aware of what it does and its consequences. This is simply due to proper awareness and not because of setting additional blockages for the user.

The same goes here too. There attacks are really new, people are getting aware of such facts. We should increase in the spreading of awareness rather than making life difficult for people.
(In reply to Girish Sharma [:Optimizer] from comment #19)
> The fact that the shortcut (rather the feature) has led to another fact that

What I meant was :

The fact that the shortcut (rather the feature) "being present for so long time" has led to ...
(In reply to Jesse Ruderman from comment #17)
> What's special about dev tools that you think we should only fix the
> security problem for a small number of sites, and leave it open everywhere
> else? We generally only do that when it's the only way to overcome severe
> compatibility problems.

We're allowing any site to trivially opt in, though. Sure, it could be fixed for everyone, but then devs would complain and we might have to make a less opaque way to enable the pref -- which then would just be fed to the people self-XSSing. Besides, self-XSS isn't too common outside of the big sites.

@Optimizer
> The same goes here too. There attacks are really new, people are getting aware of such facts. We should increase in the spreading of awareness rather than making life difficult for people.


Well, if you look at Joe's blog post linked above, he does mention that it doesn't seem possible to educate everyone on this.
I appreciate that normally with security work, the answer is to fix everything with several layers because small bugs multiply to make big holes. I argue that this issue should be treated differently because the problem is with people and not with computers.

> > I don't this that this is a totally soluble problem without fixing all
> > instances of human stupidity.
> Calling victims stupid does not solve anything.

I'm not. Sometimes people *are* stupid, and sometimes that stupidity causes them to become victims. But I am not saying that victims in general are stupid.

This is not a technicality - it's an important point. We can not totally fix this problem. Ever. There will always be someone who is more distracted and in ways that we've not thought of.

And that's one of the reasons why the thinking "fix all the problems" won't work. If we follow that thinking, we'll never win, and we will lose people who find our precautions to be obnoxious.
It might be instructive to think of the "do nothing" argument.

It's worth remembering that this is the approach taken by the Chrome and IE teams either explicitly or implicitly. I do not agree with it, hence our 2 bugs to fix the problem. I'm making this point so we understand that there are positions far more extreme that I'm taking.

---

We have only seen 3 bits of evidence when it comes to self-xss.

1. Facebook.

Facebook have a real problem, and it affects users. However Facebook's problem is in part down to their policy of allowing untrusted unvetted 3rd party plugin content to be distributed socially. It can be argued that this is a pure Facebook problem, and that having stricter controls on flash would force self-xss to be nearly twice as long, preventing them from being effective.

2. Netflix

Why are Netflix interested in hampering developer tools? I'm not aware of this being a self-xss issue. Perhaps its more about controlling users. An organization that argues for DRM might be expected to think this way.

3. Windows+R

This has been around for a very long time without exploit, which indicates to me that we don't have to make things too hard before the attack becomes unviable.

So of the 3 bits of evidence that we have, 2 point to us actively doing nothing on principle, and the other points to us doing nothing because it's someone elses problem.

---

Now, I don't totally agree with this analysis, but we should note that in order for us to think seriously about measures that annoy real people we do need more evidence that this is a serious problem.

In my opinion the landing page solution (bug 953166) tackles the problem for all users in a gentle way, and the CSP solution (bug 664589) tackles the problem in a more targetted and brutal way in areas that are specifically at risk.

It may be that these measures are not enough either immediately or over the course of an escalation in attacks, and we should reconsider this solution if that happens. If that does happen then we'll also have a way to explain our stance to the people that it annoys.
> What's special about dev tools that you think we should only fix the
> security problem for a small number of sites, and leave it open everywhere
> else?

For several reasons that I hope are now clear:

* Because we can't 100% fix the problem
* Because we don't have any evidence that it is a problem anywhere other than sites that could apply the CSP solution
* Because this fix goes too far in annoying web developers
* Because the landing page does affect all sites

It is unclear what you mean by a 'setting a pref'. Since bug 953166 (landing page) sets a pref, I assume you're talking about visiting about:config? Perhaps you could make this clear.
Note: The Facebook self-xss does not need flash. I've seen FB self-xss-es which are just a big hunk of javascript.
(In reply to Manish Goregaokar [:manishearth] from comment #25)
> Note: The Facebook self-xss does not need flash. I've seen FB self-xss-es
> which are just a big hunk of javascript.

I'm interested to know how they prime the clipboard in this case?
Also you could argue that hosting JS still comes into the 'dangerous' category.
(In reply to Joe Walker [:jwalker] from comment #26)
> (In reply to Manish Goregaokar [:manishearth] from comment #25)
> > Note: The Facebook self-xss does not need flash. I've seen FB self-xss-es
> > which are just a big hunk of javascript.
> 
> I'm interested to know how they prime the clipboard in this case?
> Also you could argue that hosting JS still comes into the 'dangerous'
> category.

In some cases, they ask users to run a script in console, which adds paste listeners [0] and then they ask them to copy some stuff and paste it somewhere in the next step.

I have also seen cases where they do not use clipboard at all. They ask users to copy the js code and paste it in console and run it (with screenshots !!! ). And then the code retrieves the friend list of the user running the code and adds a comment on the post explaining all this. There might be other cases where they do bad things with the data (like friend list or any other data). 

[0] https://developer.mozilla.org/en-US/docs/Web/API/ClipboardEvent.clipboardData
By "setting a pref", I mean checking a box that asks for the script-injection feature to be enabled. The most convenient would be to show the checkbox when it's relevant. For example, for comment 0's (B), that would be when you try to access |window| or |document| using developer tools.

It could be site-specific:
  [x] Allow my _facebook.com_ account to be hijacked if I paste malicious scripts
or global:
  [x] Allow my accounts to be hijacked if I paste malicious scripts

Since this checkbox would only be seen by web developers who use script-injection features, I think the cost would be small. But I see your argument for why the benefit might be small as well.
Bug 953166 + bug 994134 seem likely to be sufficient here, and they're more narrowly tailored, which is good for developer usability. So this seems like WONTFIX unless the other mitigations turn out to be insufficient.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → WONTFIX
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.