583262 - Security checks on f.prototype.constructor property (CheckCtorGetAccess, CheckCtorSetAccess) can die now

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

The comment on CheckCtorGetAccess says:
> * For shared precompilation of function objects, we support cloning on entry
> * to an execution context in which the function declaration or expression
> * should be processed as if it were not precompiled, where the precompiled
> * function's scope chain does not match the execution context's.  The cloned
> * function object carries its execution-context scope in its parent slot; it
> * links to the precompiled function (the "clone-parent") via its proto slot.

We don't do that anymore. Now we use the private slot.

> * Note that this prototype-based delegation leaves an unchecked access path
> * from the clone to the clone-parent's 'constructor' property.  If the clone
> * lives in a less privileged or shared scope than the clone-parent, this is
> * a security hole, a sharing hazard, or both.

I don't know how this *ever* worked. You could just say f.valueOf.call(null) and
you would have the clone-parent's global, right?

Anyway, unless I'm mistaken those checks have been unnecessary for an unsigned long long time.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 2

[Brendan Eich [:brendan]]

Oh, that is some ancient crap there...

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 461565 [details] [diff] [review]
v1

This is the reason that we needed all of those security checks in eval. cloned_fun.eval was actually the original function's scope's eval. This was fixed by bug 300079.

Comment 4

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/c6131ed87e9c

Comment 5

[Brendan Eich [:brendan]]

What a relief -- that lightened the load.

/be

Comment 6

[Robert Sayre]

backed out:

http://hg.mozilla.org/mozilla-central/rev/f7478476c9a5

caused bug 584437

Comment 7

[Jason Orendorff [:jorendorff]]

We should try this again. Bug 584565 (which caused bug 584437) was revealed by the change, not directly caused by it.

And I think that was a fluke. It's worth just one more try before ff4. If it uncovers still more tracer brokenness, then I'll hold my fire.

If you disagree, speak up now.

Comment 8

[Brendan Eich [:brendan]]

No, please, be try again. If we still need these bogo checks something is rotten in Denmark...

/be

Comment 10

[Jason Orendorff [:jorendorff]]

Waldo, feel free to take this and run with it. I'm off today or I'd do it myself.

Comment 11

[Jeff Walden [:Waldo]]

I pushed the patch to try several days ago, nothing broke, so I just pushed to TM:

http://hg.mozilla.org/tracemonkey/rev/d3f5185f0392

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d3f5185f0392