Closed Bug 583262 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: jorendorff, Assigned: jorendorff)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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.
Attached patch v1Splinter Review
Assignee: general → jorendorff
Attachment #461565 - Flags: review?(mrbkap)
Oh, that is some ancient crap there...

/be
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.
Attachment #461565 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/tracemonkey/rev/c6131ed87e9c
Whiteboard: [fixed-in-tracemonkey]
What a relief -- that lightened the load.

/be
backed out:

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

caused bug 584437
Whiteboard: [fixed-in-tracemonkey]
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.
No, please, be try again. If we still need these bogo checks something is rotten in Denmark...

/be
Duplicate of this bug: 609722
Waldo, feel free to take this and run with it. I'm off today or I'd do it myself.
I pushed the patch to try several days ago, nothing broke, so I just pushed to TM:

http://hg.mozilla.org/tracemonkey/rev/d3f5185f0392
Whiteboard: fixed-in-tracemonkey
Target Milestone: --- → mozilla2.0b8
Version: Other Branch → Trunk
http://hg.mozilla.org/mozilla-central/rev/d3f5185f0392
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.