New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shim: Function constructor can be tricked into executing the function too early #193

Closed
matt- opened this Issue Jan 14, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@matt-
Copy link

matt- commented Jan 14, 2019

attn @warner

@warner

This comment has been minimized.

Copy link
Collaborator

warner commented Jan 14, 2019

Matt described a low-impact security bug in the shim to us recently. We've got a fix ready to push, but we're waiting to coordinate with some other potential users before we disclose everything. We'll probably push the fix (and the description) tomorrow.

warner added a commit that referenced this issue Jan 15, 2019

fix sandbox bypass in Function constructor
The `Function(arg1, arg2, body)` constructor works by building a function
definition out of its parameters and then evaluating the generated string. It
is supposed to inspect the parameters to prevent access to global variables,
but Matt Austin found a clever bypass involving back-to-back template
literals.

This allowed execution of arbitrary code in the "safe" global environment,
giving `Function()` authority comparable to our safe `eval()` facility. This
wouldn't give a child Realm any additional power, but if the Realm was using
`Function()` to test that a given untrusted string was a well-formed function
body without actually executing it, this bug would give the source of that
string a surprising temporal power. The attacker's expression would be
executed immediately, whereas function constructors do not execute the
generated function at all (they merely produce a callable value which could
be invoked later).

The attacker's expression can also access `this`, which function bodies
cannot normally reach, but since functions can simply do `eval("this")`, this
is not a net increase in power.

Our fix is to reject all but the simplest of function parameters. We removed
default parameters, destructuring assignment, and the "rest"
parameter (`foo(arg1, ...rest)`), and only allow simple ASCII-based
identifier names.

refs #193
@warner

This comment has been minimized.

Copy link
Collaborator

warner commented Jan 15, 2019

The Function(arg1, arg2, body) constructor works by building a function
definition out of its parameters and then evaluating the generated string. It
is supposed to inspect the parameters to prevent access to global variables,
but Matt Austin (@matt-) found a clever bypass involving back-to-back template
literals.

Function("arg=`", "/*body`){});({x: this/**/")

which would turn into:

    (function(arg=`
    /*``*/){
     /*body`){});({x: this/**/
    })

which parses as a default argument of:

`\n/*``*/){\n/*body`

which is a pair of template literals back-to-back (so the first one nominally
evaluates to the parser to use on the second one), which can't actually
execute (because the first literal evals to a string, which can't be a parser
function), but that doesn't matter because the function is bypassed entirely.
When that gets evaluated, it defines (but does not invoke) a function, then
evaluates a simple {x: this} expression, giving access to the safe global.

This allowed execution of arbitrary code in the "safe" global environment,
giving Function() authority comparable to our safe eval() facility. This
wouldn't give a child Realm any additional power, but if the Realm was using
Function() to test that a given untrusted string was a well-formed function
body without actually executing it, this bug would give the source of that
string a surprising temporal power. The attacker's expression would be
executed immediately, whereas function constructors do not execute the
generated function at all (they merely produce a callable value which could
be invoked later).

The attacker's expression can also access this, which function bodies
cannot normally reach, but since functions can simply do eval("this"), this
is not a net increase in power.

Our fix is to reject all but the simplest of function parameters. We removed
default parameters, destructuring assignment, and the "rest"
parameter (foo(arg1, ...rest)), and only allow simple ASCII-based
identifier names.

@warner warner closed this Jan 15, 2019

@warner warner changed the title undisclosed security bug shim: Function constructor can be tricked into executing the function too early Jan 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment