Making unsafeWindow safer (by making the GM API unaccessible)

43 views
Skip to first unread message

Olivier Cornu

unread,
Feb 19, 2009, 6:11:20 PM2/19/09
to greasemo...@googlegroups.com
Hi all,

Among security-oriented discussions that took place on the FreeNode
#greasemonkey channel today, "some random guy" -- nicknamed mauke ;) --
proposed what appears to be an interesting protection against Sandbox
hijacking following blind unsafeWindow usage.
I'd like to present it to you here for discussion and review, as i'm no
javascript guru and have not followed previous security related threads.


1. Typical exploit

The typical exploit we're talking about is the one first mentioned by
Anthony in [1] -- slightly modified to make it work with current GM release:

http://evil.com/:
<script type="text/javascript">
__defineGetter__('foo', trap);
function trap() {
var gmSandbox = trap.caller.caller.__parent__;
gmSanbox.GM_log("pwned");
// do other evil things with gmSandbox
}
</script>

Userscript:
// @name Vulnerable
// @include http://evil.com/
unsafeWindow.foo;

I believe this is pretty much a typical unsafeWindow privilege
escalation case, especially regarding how a reference to the Sandbox is
acquired through ".caller.caller.__parent__". I suppose there may be
other ways to reach the same goal that i don't know of -- but i'm very
curious about. :)


2. Source of the problem

The source of the problem lies in the fact that Components.utils.Sandbox
is a javascript Object, which means all its members are accessible once
you get a reference back to the Sandbox. It is true for script-defined
variables and functions, as well as GM provided ones.


3. Proposed solution

The proposed protection deals with this problem by using a Function,
leveraging the fact that javascript function variables and
inner-functions are private. It makes them inaccessible from the hostile
environment (even if it acquires a reference to the Sandbox).

The solution needs several steps:
1. wrap the script content inside a function (as we usually do)
2. copy the GM API Sandbox references inside this function
3. delete the Sandbox references

Example userscript:
// @name GM_log protected
// @include http://evil.com/
(function sandbox() {
var GM_log = sandbox.caller.__parent__.GM_log;
delete sandbox.caller.__parent__.GM_log;
GM_log("works");
})();
GM_log("works not"); // throws "GM_log is not defined"


4. GM core implementation

We cannot do the needed work "directly" from the GM core code because
our target function variables are unreachable from there.
What we can do however, is prepend the necessary code to the script
content before we call evalInSandbox().

Example, greasemonkey.js:injectScripts():
scriptSrc = "(function sandbox() {\
var GM_log = sandbox.caller.__parent__.GM_log;\
delete sandbox.caller.__parent__.GM_log;\
" + scriptSrc + "})()";
this.evalInSandbox(scriptSrc, url, sandbox, script);

Of course, we need to do that not only for GM_log, but for every Sandbox
member.


Up for comments.

--
Olivier


[1] http://bit.ly/LRjk

Johan Sundström

unread,
Feb 19, 2009, 8:35:15 PM2/19/09
to greasemo...@googlegroups.com
On Thu, Feb 19, 2009 at 3:11 PM, Olivier Cornu <o.c...@gmail.com> wrote:
> 1. Typical exploit
>
> The typical exploit we're talking about is the one first mentioned by
> Anthony in [1] -- slightly modified to make it work with current GM release:
>
> http://evil.com/:
> <script type="text/javascript">
> __defineGetter__('foo', trap);
> function trap() {
> var gmSandbox = trap.caller.caller.__parent__;
> gmSanbox.GM_log("pwned");
> // do other evil things with gmSandbox
> }
> </script>
>
> Userscript:
> // @name Vulnerable
> // @include http://evil.com/
> unsafeWindow.foo;

Quick question: have you tested that your, purportedly-protected,
version of above unsafe userscript,

// @name GM_log protected
// @include http://evil.com/
(function sandbox() {
var GM_log = sandbox.caller.__parent__.GM_log;
delete sandbox.caller.__parent__.GM_log;

unsafeWindow.foo;
})();

...does not yield a "pwned" log line with the example setup (i e: that
this fix works), as the original example did?

If it indeed does fix it, I think I have some detail adjustment to the
proposal that makes it work for @unwrap too.

--
/ Johan Sundström, http://ecmanaut.blogspot.com/

Aaron Boodman

unread,
Feb 19, 2009, 8:38:09 PM2/19/09
to greasemo...@googlegroups.com
On Thu, Feb 19, 2009 at 3:11 PM, Olivier Cornu <o.c...@gmail.com> wrote:
> The typical exploit we're talking about is the one first mentioned by
> Anthony in [1] -- slightly modified to make it work with current GM release:
>
> http://evil.com/:
> <script type="text/javascript">
> __defineGetter__('foo', trap);
> function trap() {
> var gmSandbox = trap.caller.caller.__parent__;
> gmSanbox.GM_log("pwned");
> // do other evil things with gmSandbox
> }
> </script>
>
> Userscript:
> // @name Vulnerable
> // @include http://evil.com/
> unsafeWindow.foo;
>
> I believe this is pretty much a typical unsafeWindow privilege
> escalation case, especially regarding how a reference to the Sandbox is
> acquired through ".caller.caller.__parent__". I suppose there may be
> other ways to reach the same goal that i don't know of -- but i'm very
> curious about. :)

This doesn't work for me. I can't get a reference to the sandbox. I
think Mozilla has put stuff in place recently that prevents it, even
before it gets to Greasemonkey's own check.

- a

Olivier Cornu

unread,
Feb 19, 2009, 8:39:42 PM2/19/09
to greasemo...@googlegroups.com
A few corrections...

Olivier Cornu a écrit :

There's a mistake here, i should have written: "it is true for GM
provided variables and functions, not for script-defined ones (unless
@unwrap is used)".

>
> 3. Proposed solution
>
> The proposed protection deals with this problem by using a Function,
> leveraging the fact that javascript function variables and
> inner-functions are private. It makes them inaccessible from the hostile
> environment (even if it acquires a reference to the Sandbox).
>
> The solution needs several steps:
> 1. wrap the script content inside a function (as we usually do)
> 2. copy the GM API Sandbox references inside this function
> 3. delete the Sandbox references
>
> Example userscript:
> // @name GM_log protected
> // @include http://evil.com/
> (function sandbox() {
> var GM_log = sandbox.caller.__parent__.GM_log;
> delete sandbox.caller.__parent__.GM_log;
> GM_log("works");
> })();
> GM_log("works not"); // throws "GM_log is not defined"

As we wrap the script content inside an anonymous function as a default
(unless @unwrap is used), a simpler example would be:


// @name GM_log protected
// @include http://evil.com/

var ref = {}.__parent__;
var GM_log = ref.GM_log;
delete ref.GM_log;
GM_log("works");

However, proceeding like this would not allow to show the Sandbox GM_log
is not reachable anymore after it's been deleted (unless Evil script was
listing all accessible Sandbox members).

>
> 4. GM core implementation
>
> We cannot do the needed work "directly" from the GM core code because
> our target function variables are unreachable from there.
> What we can do however, is prepend the necessary code to the script
> content before we call evalInSandbox().
>
> Example, greasemonkey.js:injectScripts():
> scriptSrc = "(function sandbox() {\
> var GM_log = sandbox.caller.__parent__.GM_log;\
> delete sandbox.caller.__parent__.GM_log;\
> " + scriptSrc + "})()";
> this.evalInSandbox(scriptSrc, url, sandbox, script);

Same as above:
scriptSrc = "(function(){\
var ref = {}.__parent__;\
var GM_log = ref.GM_log;\
delete ref.GM_log;\
"+ scriptSrc +"})()";

>
> Of course, we need to do that not only for GM_log, but for every Sandbox
> member.

--
Olivier

Olivier Cornu

unread,
Feb 19, 2009, 8:44:07 PM2/19/09
to greasemo...@googlegroups.com
Johan Sundström a écrit :

> Quick question: have you tested that your, purportedly-protected,
> version of above unsafe userscript,
>
> // @name GM_log protected
> // @include http://evil.com/
> (function sandbox() {
> var GM_log = sandbox.caller.__parent__.GM_log;
> delete sandbox.caller.__parent__.GM_log;
> unsafeWindow.foo;
> })();
>
> ...does not yield a "pwned" log line with the example setup (i e: that
> this fix works), as the original example did?

Yes, i did. As written, it throwns an "undefined" error.
In fact, i've also used an "Evil script" listing all members from the
acquired Sandbox reference. GM_log is no there anymore.

> If it indeed does fix it, I think I have some detail adjustment to the
> proposal that makes it work for @unwrap too.

Please shoot. :)

--
Olivier

Olivier Cornu

unread,
Feb 19, 2009, 8:46:19 PM2/19/09
to greasemo...@googlegroups.com
Aaron Boodman a écrit :

That's surprising... :|
I'm running FF 3.0.6. Which one are you using?

--
Olivier

Olivier Cornu

unread,
Feb 19, 2009, 8:53:38 PM2/19/09
to greasemo...@googlegroups.com
Olivier Cornu a écrit :

> Johan Sundström a écrit :
>> Quick question: have you tested that your, purportedly-protected,
>> version of above unsafe userscript,
>>
>> // @name GM_log protected
>> // @include http://evil.com/
>> (function sandbox() {
>> var GM_log = sandbox.caller.__parent__.GM_log;
>> delete sandbox.caller.__parent__.GM_log;
>> unsafeWindow.foo;
>> })();
>>
>> ...does not yield a "pwned" log line with the example setup (i e: that
>> this fix works), as the original example did?
>
> Yes, i did. As written, it throwns an "undefined" error.

Sorry, my mistake: access from Evil script after GM_log has been deleted
throws an "is not a function" error.
"undefined" is the error raised from the script itself, outside sandbox().

--
Olivier

Johan Sundström

unread,
Feb 19, 2009, 9:02:59 PM2/19/09
to greasemo...@googlegroups.com
On Thu, Feb 19, 2009 at 5:44 PM, Olivier Cornu <o.c...@gmail.com> wrote:
> Johan Sundström a écrit :
>> Quick question: have you tested that your, purportedly-protected,
>> version of above unsafe userscript,
>>
>> // @name GM_log protected
>> // @include http://evil.com/
>> (function sandbox() {
>> var GM_log = sandbox.caller.__parent__.GM_log;
>> delete sandbox.caller.__parent__.GM_log;
>> unsafeWindow.foo;
>> })();
>>
>> ...does not yield a "pwned" log line with the example setup (i e: that
>> this fix works), as the original example did?
>
> Yes, i did. As written, it throwns an "undefined" error.
> In fact, i've also used an "Evil script" listing all members from the
> acquired Sandbox reference. GM_log is no there anymore.

Good (as indeed it should not). If the security breech reproduces
pre-fix, and does not post-fix, I'm +1 proceeding with something along
the lines of this, and possibly taking out the current fix (which
broke some things by forcing people to go to js-wizard-level closure
dribbling to do some things that were easy before -- the GM wiki has
all the details).

>> If it indeed does fix it, I think I have some detail adjustment to the
>> proposal that makes it work for @unwrap too.
>
> Please shoot. :)

Pseudocode, as I'm a bit busy at the moment:

GM side, before:
sandbox.GM_log = GM_hitch(logger, "log");
sandbox.console = console;
...

GM side, after:
sandbox.GM = {
GM_log: GM_hitch(logger, "log"),
console: console,
// ...
};

Sandbox lead-in side, after:
GM_log = GM.GM_log;
console = GM.console;
// ...
delete GM;

Johan Sundström

unread,
Feb 19, 2009, 9:06:42 PM2/19/09
to greasemo...@googlegroups.com
Hmm. On second thought, it is quite probable that this approach does
not fix the breech in @unwrap mode, in which case: pardon the noise.
(Can't hurt testing it, though.)

Olivier Cornu

unread,
Feb 19, 2009, 10:32:40 PM2/19/09
to greasemo...@googlegroups.com
Johan Sundström a écrit :

> Good (as indeed it should not). If the security breech reproduces
> pre-fix, and does not post-fix, I'm +1 proceeding with something along
> the lines of this, and possibly taking out the current fix (which
> broke some things by forcing people to go to js-wizard-level closure
> dribbling to do some things that were easy before -- the GM wiki has
> all the details).

I'm not sure what you mean here. Are you talking about [1] and the call
stack checking code?

--
Olivier


[1] http://wiki.greasespot.net/0.7.20080121.0_compatibility

Anthony Lieuallen

unread,
Feb 19, 2009, 10:37:13 PM2/19/09
to greasemo...@googlegroups.com
On 2/19/2009 6:11 PM, Olivier Cornu wrote:
> ...
> Up for comments.

A ticket to track this in would be very good. A branch with a proposed
implementation(s) of this solution (it seems there already is one, or
more) would similarly be very good. There's a lot of abbreviations in
the thread here, I'd love to see Olivier's original, and Johan's
(sandbox.GM={...}) version .. is either one noticeably shorter or
simpler, once complete?

And do I understand: if implemented (exclusively) this would make
@unwrap revert unsafeWindow to its current level of unsafe-ness?

Johan Sundström

unread,
Feb 20, 2009, 1:20:24 AM2/20/09
to greasemo...@googlegroups.com
On Thu, Feb 19, 2009 at 7:37 PM, Anthony Lieuallen <aran...@gmail.com> wrote:
> On 2/19/2009 6:11 PM, Olivier Cornu wrote:
>> ...
>> Up for comments.
>
> A ticket to track this in would be very good. A branch with a proposed
> implementation(s) of this solution (it seems there already is one, or
> more) would similarly be very good. There's a lot of abbreviations in
> the thread here, I'd love to see Olivier's original, and Johan's
> (sandbox.GM={...}) version .. is either one noticeably shorter or
> simpler, once complete?

+1; tickets are better than the mailing list to track this.

> And do I understand: if implemented (exclusively) this would make
> @unwrap revert unsafeWindow to its current level of unsafe-ness?

If my suspicions prove right (I'm afraid I'm a bit too preoccupied to
do the research), it would.

If so, it is wise to keep the call-stack peeking safe-guards in place.
If not, I'd be +0 to +1 about removing them (more, the more you feel
similarly).

Aaron Boodman

unread,
Feb 20, 2009, 1:28:58 AM2/20/09
to greasemo...@googlegroups.com
On Thu, Feb 19, 2009 at 5:46 PM, Olivier Cornu <o.c...@gmail.com> wrote:
> That's surprising... :|
> I'm running FF 3.0.6. Which one are you using?

Same, with Greasemonkey 0.8.20090123.1.

I thought that last time I tried this trick, it was not possible to
delete members from sandbox objects. If it is now, then great. Also, I
think we need to keep the call-stack check in because it is still easy
for gm devs or an author to accidentally expose a reference to a gm
API to content script.

- a

Olivier Cornu

unread,
Feb 20, 2009, 7:17:19 AM2/20/09
to greasemo...@googlegroups.com
Aaron Boodman a écrit :

> Also, I
> think we need to keep the call-stack check in because it is still easy
> for gm devs or an author to accidentally expose a reference to a gm
> API to content script.

I agree... unfortunately (the sheer amount of tricks we have to go
through to contain javascript's "flexibility" just roughly is rather
insane).

--
Olivier

Olivier Cornu

unread,
Feb 20, 2009, 1:31:13 PM2/20/09
to greasemo...@googlegroups.com
Anthony Lieuallen a écrit :

> A ticket to track this in would be very good.

Your wishes are my command: ticket #225 [1]. :)
I even included a nice evil.html demo page [2], with user-selectable
exploits and full sandbox dumps (before and after exploitation). ;)

> A branch with a proposed
> implementation(s) of this solution (it seems there already is one, or
> more) would similarly be very good.

The fix can fit in a single small patch. Do you really think it's
necessary to create a new branch to deal with this?

> And do I understand: if implemented (exclusively) this would make
> @unwrap revert unsafeWindow to its current level of unsafe-ness?

I'm not sure i understand what you mean here (but i'll give it a shot
anyway)... :p
The proposed fix is entirely based on the privateness of function
members, thus an @unwrap script (which has no GM-core provided wrapping
function) cannot be protected this way, unfortunately. So, nothing
changes for @unwrapped scripts... :/

--
Olivier


[1] http://greasemonkey.devjavu.com/ticket/225
[2]
http://greasemonkey.devjavu.com/attachment/ticket/225/evil.html?format=raw

Anthony Lieuallen

unread,
Feb 20, 2009, 1:55:48 PM2/20/09
to greasemo...@googlegroups.com
On 2/20/2009 1:31 PM, Olivier Cornu wrote:
>> A branch with a proposed implementation(s) of this solution (it
>> seems there already is one, or more) would similarly be very good.
> The fix can fit in a single small patch. Do you really think it's
> necessary to create a new branch to deal with this?

Yes, I want to be able to build, run, and test the proposed fixes. A
branch is an easy way to do that. (Maybe I'm dim, but applying patches
often causes problems for me. And I want to make sure the code I test
is exactly that proposed, or else the test might be invalid.)

Olivier Cornu

unread,
Feb 20, 2009, 5:00:27 PM2/20/09
to greasemo...@googlegroups.com
Anthony Lieuallen a écrit :

> On 2/20/2009 1:31 PM, Olivier Cornu wrote:
>>> A branch with a proposed implementation(s) of this solution (it
>>> seems there already is one, or more) would similarly be very good.
>> The fix can fit in a single small patch. Do you really think it's
>> necessary to create a new branch to deal with this?
>
> Yes, I want to be able to build, run, and test the proposed fixes. A
> branch is an easy way to do that.

The new branch is called "saferapi". Patch has been committed.
Afaict it properly hides the GM API elements, according to my tests.
Further tests are more than welcome.

It does not mean we're out of trouble though. :(
While looking into this issue, i believe i discovered new (not directly
GM-related) Sandbox issues...

It appears that, if your script uses a js core function -- like alert()
-- it will be added to the Sandbox (wrapped into a XPCNativeWrapper):
sandbox = {
(...)
alert: function XPCNativeWrapper function wrapper() {
[native code]
}
}
So, not only is/was it possible to hijack GM stuff, but it's also
possible to hijack any javascript core functions a script has used once!

Even worse: if you set Sandbox.alert to null from the hostile
environment *before* your script uses alert() for the first time (that
is before any alert Sandbox member exists), the Sandbox won't be able to
import the wrapped js core function (i.e. it won't replace even a null
value with it)!
In other words, you can _preventively_ hijack (or just nullify) any js
core function a script can have access to. Pretty bad. :/
That very much looks like a Components.utils.Sandbox bug to me.

Whatever, this is getting really insane...

--
Olivier

Aaron Boodman

unread,
Feb 20, 2009, 5:55:02 PM2/20/09
to greasemo...@googlegroups.com
On Fri, Feb 20, 2009 at 2:00 PM, Olivier Cornu <o.c...@gmail.com> wrote:
> It does not mean we're out of trouble though. :(
> While looking into this issue, i believe i discovered new (not directly
> GM-related) Sandbox issues...
>
> It appears that, if your script uses a js core function -- like alert()
> -- it will be added to the Sandbox (wrapped into a XPCNativeWrapper):
> sandbox = {
> (...)
> alert: function XPCNativeWrapper function wrapper() {
> [native code]
> }
> }
> So, not only is/was it possible to hijack GM stuff, but it's also
> possible to hijack any javascript core functions a script has used once!
>
> Even worse: if you set Sandbox.alert to null from the hostile
> environment *before* your script uses alert() for the first time (that
> is before any alert Sandbox member exists), the Sandbox won't be able to
> import the wrapped js core function (i.e. it won't replace even a null
> value with it)!
> In other words, you can _preventively_ hijack (or just nullify) any js
> core function a script can have access to. Pretty bad. :/
> That very much looks like a Components.utils.Sandbox bug to me.

Right, the core problem is that unsafeWindow creates a bridge into the
sandbox. From there you can expect all kinds of silly things to
happen, but most of them are namespace collision types of things. This
is just an avoidable fact of life as long as there is a bridge between
user scripts and content scripts.

The real thing we don't want to happen is for content to be able to
use this bridge to get elevated capabilities.

- a

BD

unread,
Feb 20, 2009, 6:35:54 PM2/20/09
to greasemo...@googlegroups.com
Does that mean that, if any GmScript does something bad that allows access into the sandbox, that everyone's scripts are compromised?

Olivier Cornu

unread,
Feb 20, 2009, 6:39:33 PM2/20/09
to greasemo...@googlegroups.com
Aaron Boodman a écrit :

> On Fri, Feb 20, 2009 at 2:00 PM, Olivier Cornu <o.c...@gmail.com> wrote:
>> So, not only is/was it possible to hijack GM stuff, but it's also
>> possible to hijack any javascript core functions a script has used once!

My mistake: alert() is not a js core function, but a window method.

> The real thing we don't want to happen is for content to be able to
> use this bridge to get elevated capabilities.

You're right, privilege escalation is the real deal. Although i believe
existing call-stack checking in risky GM_* methods takes care of it
quite well.

What this fix attempts to do is insuring a proper environment to the
script (making sure it will always have working references available to
window, document, GM_*() functions, etc).
In this regard, it may be useful to provide a similar insurance for the
other sandbox members as well: the XPC-wrapped window methods and
properties.

--
Olivier

Olivier Cornu

unread,
Feb 20, 2009, 6:41:30 PM2/20/09
to greasemo...@googlegroups.com
BD a écrit :

> Does that mean that, if any GmScript does something bad that allows
> access into the sandbox, that everyone's scripts are compromised?

No, each script has its own sandbox.

--
Olivier

Olivier Cornu

unread,
Feb 21, 2009, 9:11:35 AM2/21/09
to greasemo...@googlegroups.com
Olivier Cornu a écrit :

> What this fix attempts to do is insuring a proper environment to the
> script (making sure it will always have working references available to
> window, document, GM_*() functions, etc).
> In this regard, it may be useful to provide a similar insurance for the
> other sandbox members as well: the XPC-wrapped window methods and
> properties.

/Addendum/:

With the proposed fix in its current state, these window members are
protected if they are referenced _explicitly_, as in: window.alert().
Whereas the shortcut implicit versions like alert() are not.
Thus, if this fix was merged in trunk, newly published (or updated)
scripts could use this explicit notation *exclusively* in order to gain
a fully-working environment, even if they use blind unsafeWindow
operations in hostile web pages. However, scripts that use shortcut
implicit versions, which i'm afraid is the common case, would not be
protected.
In other words, the provided protection (as it currently is) is not
fully backward-compatible: it should not break anything, but will not
fix all existing code either.

After looking deeper into this issue, i guess i've found the reason why
alert() does not show in the sandbox before it's been used by the script.
Excerpt from [1], limitations of XPCNativeWrapper:
13. Enumerating the properties of an XPCNativeWrapper via "for (var p
in wrapper)" does not enumerate the IDL-defined properties.
That's indeed how the current saferapi branch fix processes,
greasemonkey.js:299:
for (var i in this) {
I'm working on a new fix taking this into account.

--
Olivier


PS: In all the above it is assumed the proposed fix indeed achieves what
it means to. This has not (yet) been proven true (or agreed on).

[1] https://developer.mozilla.org/en/XPCNativeWrapper

Olivier Cornu

unread,
Feb 27, 2009, 12:58:18 PM2/27/09
to greasemo...@googlegroups.com
Olivier Cornu a écrit :

> The proposed protection deals with this problem by using a Function,
> leveraging the fact that javascript function variables and
> inner-functions are private. It makes them inaccessible from the hostile
> environment (even if it acquires a reference to the Sandbox).
>
> The solution needs several steps:
> 1. wrap the script content inside a function (as we usually do)
> 2. copy the GM API Sandbox references inside this function
> 3. delete the Sandbox references

Follow-up.

So, after quite a bit of tinkering and testing with multiple
combinations of caller/callee/__parent__/__proto__/prototype and the
likes, it appears that the proposed solution, as implemented in [1], is
working quite well (confirmation/contradiction from a tiers would be
fantastic):
- after being copied in vars and removed from the sandbox, the GM API
is unreachable from unsafeWindow (either via a direct call to an
unsafeWindow function, or unsafeWindow property booby-trapping).
- scripts vars and functions are unreachable too, unless they are
defined with 'this.prop' or 'this.method' (following an unfortunate
ECMAScript spec, 'this' refers to the sandbox).

The only things accessible are the sandbox itself and, through it, its
IDL-defined members. As a consequence, they can be overwritten from
unsafeWindow.
Yet, replacing a legit sandbox member with a malicious piece of code
will not fool the call-stack checker. In other words, it will _not_
grant that piece of code userscript privileges.
The call-stack checker (GM_apiLeakCheck) definitely is an avoidable and
very valuable part of GM's security (thanks Anthony).

The only "attack vector" for page content is messing with sandbox IDL
members which, at worst, would result in hurting the script ability to
customize the page. However, this is only true for scripts using
implicit (shortcut) references to these IDL members (e.g. alert()),
because they refer to sandbox members; using explicit references (e.g.
window.alert()) is enough, because window is a distinct XPCNV protected
by [1].

As a preliminary conclusion, [1] and GM_apiLeakCheck provide a way for a
script to deal with unsafeWindow at will, without any precaution, and so
without compromising the script code, its ability to customize the page
or offering opportunities to page content for privilege escalation.
In other words, provided you:
- only use explicit references to window members
- don't rely on members you've added to the sandbox with top-level
'this.*' statements (which is a _silly_ thing to do in the first place)
...unsafeWindow basically is *safe*.

All the above statements only hold true when @unwrap is _not_ used.
@unwrap puts your script at the mercy of page content as soon as you
touch unsafeWindow. It is a dangerous feature and i personally don't
really understand why it's there to begin with (explanations welcome),
so i'd simply suggest deprecating it and be done with all the "avoiding
unsafeWindow" tricks that the wiki is filled with, once and for all.

Johan Sundström

unread,
Feb 27, 2009, 3:35:56 PM2/27/09
to greasemo...@googlegroups.com
On Fri, Feb 27, 2009 at 9:58 AM, Olivier Cornu <o.c...@gmail.com> wrote:
> Olivier Cornu a écrit :
>> The proposed protection deals with this problem by using a Function,
>> leveraging the fact that javascript function variables and
>> inner-functions are private. It makes them inaccessible from the hostile
>> environment (even if it acquires a reference to the Sandbox).
>>
>> The solution needs several steps:
>>   1. wrap the script content inside a function (as we usually do)
>>   2. copy the GM API Sandbox references inside this function
>>   3. delete the Sandbox references
>
> Follow-up.
>
> So, after quite a bit of tinkering and testing with multiple
> combinations of caller/callee/__parent__/__proto__/prototype and the
> likes, it appears that the proposed solution, as implemented in [1], is
> working quite well (confirmation/contradiction from a tiers would be
> fantastic):
>   - after being copied in vars and removed from the sandbox, the GM API
> is unreachable from unsafeWindow (either via a direct call to an
> unsafeWindow function, or unsafeWindow property booby-trapping).

Excellent.

>   - scripts vars and functions are unreachable too, unless they are
> defined with 'this.prop' or 'this.method' (following an unfortunate
> ECMAScript spec, 'this' refers to the sandbox).

I'd rather phrase that as "script identifiers existing as properties
on the global object [=== this === the sandbox] remain visible to this
attack vector; others do not". Just for reference, this surmises the
currently known attack on the Greasemonkey sandbox from page content
land. The reason it was a problem was because it exposed the GM_* APIs
to attackers, nothing else (in the typical case of a script that does
not do what you write above, or use @unwrap). As best we know, that
attack is thwarted (for the same typical case) by your fix.

That is good. We can't know that it equates "unsafeWindow is safe",
though, nor, even if it happens to be true right now, that it will
remain so indefinitely; Mozilla is a moving target often changing
underfoot, sometimes exposing new threats.

Livable; I know of no (Greasemonkey-centricly, anyway) practical
reason for writing one's user script that way, so it is at least
unlikely to be prevalent in the wild.

> The only things accessible are the sandbox itself and, through it, its
> IDL-defined members. As a consequence, they can be overwritten from
> unsafeWindow.

Unfortunate, but at least not a security regression from before your
fix, as far as I understand it.

Put differently, I assume the above means "If your script accesses
unsafeWindow, some methods/objects of the BOM like alert and
XPathResult may not be what they should", and, given other context,
that a script that is afraid of it and wants to apply protective
measures, can successfully do so by using window.alert,
window.XPathResult and others.

> The call-stack checker (GM_apiLeakCheck) definitely is an avoidable and
> very valuable part of GM's security (thanks Anthony).

I think you meant "unavoidable" here?

> In other words, provided you:
>   - only use explicit references to window members
>   - don't rely on members you've added to the sandbox with top-level
> 'this.*' statements (which is a _silly_ thing to do in the first place)
> ...unsafeWindow basically is *safe*.

...from page content gaining access to identifiers in the script or
Greasemonkey via this known attack vector.

Script authors that use @unwrap should know that their script's top
level identifiers do trigger the second clause. (And may choose to
note, or ignore, that Olivier considers that a silly thing to do. :-)

> All the above statements only hold true when @unwrap is _not_ used.

(Just FYI, I did read this sentence; my narration above just tries to
make people that skim without _great_ attention to the fine print, not
miss important security stuff.)

> @unwrap puts your script at the mercy of page content as soon as you
> touch unsafeWindow. It is a dangerous feature and i personally don't
> really understand why it's there to begin with (explanations welcome),

In the quest for clear communication and "placing blame" where it is
due, @unwrap is a mechanism that puts identifiers on the global
object, and putting identifiers on the global object is what makes
them visible to this attack. We can't guard against people doing so
(this.x = something is still doable), so removing @unwrap would not
close and seal anything, only kill a useful feature to those that use
it.

What @unwrap does, from three points of view: how it works, what its
mechanism is, and what is gained by using it:

@unwrap removes the preceding "(function(){" and terminating "})();"
that Greasemonkey surrounds your script with before evaluating it, if
you have this metadata imperative in your script header when the
script is installed.

@unwrap makes all your script's top-level identifiers members of the
global object (this).

@unwrap provides reflection
(http://en.wikipedia.org/wiki/Reflection_(computer_science)).

When I introduced it, I described it like this:
http://groups.google.com/group/greasemonkey-dev/browse_thread/thread/03a470ba89d1bd7c#msg_d611707c40003326

...but the Wikipedia article about what reflection is good for is a
much better and richer source. As for yourself, you will likely find
this example much more interesting, when we have @depend:

// ==UserScript==
// @unwrap
// ==/UserScript==

GM_export = this;

// tons of code here, _everything_ exported to any script that @depend:s on us

...which will be(come) what I use it for most myself, in the probably
rather near future.

> so i'd simply suggest deprecating it and be done with all the "avoiding
> unsafeWindow" tricks that the wiki is filled with, once and for all.

-1 on both. People that share your comfort about using unsafeWindow
while taking care not to trigger any of the gotchas above may of
course also choose to not bother using any of the unsafeWindow tricks
in the wiki.

Olivier Cornu

unread,
Feb 27, 2009, 6:02:08 PM2/27/09
to greasemo...@googlegroups.com
Johan Sundström a écrit :

> On Fri, Feb 27, 2009 at 9:58 AM, Olivier Cornu <o.c...@gmail.com> wrote:
>> it appears that the proposed solution, as implemented in [1], is
>> working quite well (confirmation/contradiction from a tiers would be
>> fantastic):

In fact, i'm afraid a formal proof (or confirmation) is just unreachable
-- at least by me, at the moment. So that it seems reasonable to
consider it true until proven otherwise (exploit), from a pragmatic
standpoint.

> That is good. We can't know that it equates "unsafeWindow is safe",
> though

What would you require to call it "safe"?

>> The only things accessible are the sandbox itself and, through it, its
>> IDL-defined members. As a consequence, they can be overwritten from
>> unsafeWindow.
>
> Unfortunate, but at least not a security regression from before your
> fix, as far as I understand it.

That's right.

> Put differently, I assume the above means "If your script accesses
> unsafeWindow, some methods/objects of the BOM like alert and
> XPathResult may not be what they should"

I think you meant "the DOM" here?

>> The call-stack checker (GM_apiLeakCheck) definitely is an avoidable and
>> very valuable part of GM's security (thanks Anthony).
>
> I think you meant "unavoidable" here?

Indeed. :)

>> In other words, provided you:
>> - only use explicit references to window members
>> - don't rely on members you've added to the sandbox with top-level
>> 'this.*' statements (which is a _silly_ thing to do in the first place)
>> ...unsafeWindow basically is *safe*.
>
> ...from page content gaining access to identifiers in the script or
> Greasemonkey via this known attack vector.

Not really: "from page content hijacking of unsafeWindow usage" is the
claim. That is, until proven wrong, of course. :)
Is there any other known vector that we should protect from?

>> @unwrap puts your script at the mercy of page content as soon as you
>> touch unsafeWindow. It is a dangerous feature and i personally don't
>> really understand why it's there to begin with (explanations welcome),
>
> In the quest for clear communication and "placing blame" where it is
> due, @unwrap is a mechanism that puts identifiers on the global
> object, and putting identifiers on the global object is what makes
> them visible to this attack. We can't guard against people doing so
> (this.x = something is still doable), so removing @unwrap would not
> close and seal anything, only kill a useful feature to those that use
> it.

Your considering compromising _one_ identifier (through an ill-inspired
statement) being the same as compromising _all_ of them indistinctly
tickles my pragmatism. :)

> What @unwrap does, from three points of view: how it works, what its
> mechanism is, and what is gained by using it:
>

> @unwrap makes all your script's top-level identifiers members of the
> global object (this).

... which unfortunately happens to be the sandbox.

In the use-case you present in this thread (alternative to 'switch'),
i'd agree with Anthony: if you want to do that, just put your functions
in any object you like. It's clean reflection, without risks of
namespace conflicts or compromise from the page content. @unwrap is not
needed here.

I have nothing against the idea of full script reflection, although the
only interesting usage of it i can figure right now is debugging. In
which case a header tag, furthermore only switchable on reinstall, does
not seem to be the proper mechanism.
Even for debugging (which does not need the production safety-nets), the
fact that removing the wrapper function can introduce new bugs, like a
conflict with a Mozilla reserved name in the sandbox for example, is
problematic.
Overall, if this full-reflection feature is indeed needed (which i'm not
convinced of), doing it through removal of the wrapper function does not
seem to be the right way to achieve it, afaict.

> As for yourself, you will likely find
> this example much more interesting, when we have @depend:
>
> // ==UserScript==
> // @unwrap
> // ==/UserScript==
>
> GM_export = this;

I'm not sure at the moment this kind of construct is advisable...

--
Olivier

Olivier Cornu

unread,
Feb 28, 2009, 6:21:49 AM2/28/09
to greasemo...@googlegroups.com
Olivier Cornu a écrit :
> Johan Sundström a écrit :
>> As for yourself, you will likely find
>> this example much more interesting, when we have @depend:
>>
>> // ==UserScript==
>> // @unwrap
>> // ==/UserScript==
>>
>> GM_export = this;
>
> I'm not sure at the moment this kind of construct is advisable...

Now that i've thought about it longer, this construct is just plain
impossible: GM_export (or 'self') is an object that's instantiated in GM
core (chrome context). The proposed statement redefines GM_export in the
script context, but the "real" GM_export (the one that is stored in GM
core and will be inserted in dependent scripts) is still {}.
There is a good reason to that: security (i guess it's the proper term).

In fact, i've had a version of modularity branch allowing this kind of
construct at some point (rev 785 [1]). In other words, after the script
has been evalInSandbox, GM core would set its own GM_export to the
script-defined one.
The problem is that, using this technique, a dependent script could mess
with this "GM_export" prototype in a way that would affect every other
dependent scripts.

Rev 786 changed that [2]. GM_export is instantiated in GM core, before
evalInSandbox, and the only things a script is allowed to do are:
- populate it: GM_export.prop = ...;
- set its runtime prototype: GM_export.__proto__ = {...};
A dependent script cannot change this "GM_export" prototype in a way
that would affect another dependent script anymore.
Afaict, this is because GM_export being instantiated in chrome context,
its prototype cannot be modified from the sandbox context (it actually
throws an error).

--
Olivier


PS: I'm using 'GM_export' here to remain compliant with your post, but
the current implementation uses 'self' instead. Admittedly, there is no
consensus at this point on what the syntax should be.

[1]
http://greasemonkey.devjavu.com/changeset?old_path=%2F&old=784&new_path=%2F&new=785
[2]
http://greasemonkey.devjavu.com/changeset?old_path=%2F&old=785&new_path=%2F&new=786

Olivier Cornu

unread,
Feb 28, 2009, 6:35:13 AM2/28/09
to greasemo...@googlegroups.com
Olivier Cornu a écrit :

> Olivier Cornu a écrit :
>> Johan Sundström a écrit :
>>>
>>> // ==UserScript==
>>> // @unwrap
>>> // ==/UserScript==
>>>
>>> GM_export = this;
>
> Rev 786 changed that [2]. GM_export is instantiated in GM core, before
> evalInSandbox, and the only things a script is allowed to do are:
> - populate it: GM_export.prop = ...;
> - set its runtime prototype: GM_export.__proto__ = {...};

Admittedly, what you could do to achieve your above purpose is:
GM_export.__proto__ = this;
But then GM_export would be filled with lots of junk, namely all the
IDL-defined DOM properties of the sandbox. I'd need to run some tests to
see if the GM_export mechanism actually accepts these XPCNW sandbox
members to "jump" from one script's sandbox to another's.
Whether it does or not, i'm still reluctant at considering this an
advisable construct though...

--
Olivier

Olivier Cornu

unread,
Mar 2, 2009, 12:02:53 PM3/2/09
to greasemo...@googlegroups.com
On Sat, Feb 28, 2009 at 00:02, Olivier Cornu <o.c...@gmail.com> wrote:
> Johan Sundström a écrit :
>> When I introduced it, I described it like this:
>>
>> http://groups.google.com/group/greasemonkey-dev/browse_thread/thread/03a470ba89d1bd7c#msg_d611707c40003326
>
> In the use-case you present in this thread (alternative to 'switch'), i'd
> agree with Anthony: if you want to do that, just put your functions in any
> object you like. It's clean reflection, without risks of namespace conflicts
> or compromise from the page content. @unwrap is not needed here.
>
> I have nothing against the idea of full script reflection, although the only
> interesting usage of it i can figure right now is debugging.

As a complement of information about how useful @unwrap is in a
"production environment" (/i.e./ debugging aside) in the real world:
among the 28143 scripts hosted on userscripts.org as of february 28th,
10 use @unwrap (0.03%).
This number can be considered a high estimate, as i wouldn't be
surprised that a review of those 10 scripts would show at least some
of them actually don't need @unwrap.

It can be compared to the number of scripts using the currently
unsupported @version header tag in the same sample: 4861 (17.3%).

I believe these numbers speak for themselves... :)

--
Olivier

Aaron Boodman

unread,
Mar 2, 2009, 1:30:31 PM3/2/09
to greasemo...@googlegroups.com
On Mon, Mar 2, 2009 at 9:02 AM, Olivier Cornu <o.c...@gmail.com> wrote:
> As a complement of information about how useful @unwrap is in a
> "production environment" (/i.e./ debugging aside) in the real world:
> among the 28143 scripts hosted on userscripts.org as of february 28th,
> 10 use @unwrap (0.03%).
> This number can be considered a high estimate, as i wouldn't be
> surprised that a review of those 10 scripts would show at least some
> of them actually don't need @unwrap.
>
> It can be compared to the number of scripts using the currently
> unsupported @version header tag in the same sample: 4861 (17.3%).
>
> I believe these numbers speak for themselves... :)

Heheh.

- a

Reply all
Reply to author
Forward
0 new messages