466836 - AutoScriptEvaluate doesn't set aside termination functions at evaluation start

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

AutoScriptEvaluate (in xpcwrappedjsclass.cpp) calls ScriptExecuted() at evaluation end, which calls ScriptEvaluated if not mContext->fp.

The thing is, if someone screws up and sets a termination function on a context that's not running script, this could hide the resulting bugs, introducing changes into what web content sees depending on exactly what calls C++ makes or doesn't make into chrome JS (in particular, in bug 466826, whether SetStatus() is implemented in JS or not).

I think evaluation start in AutoScriptEvaluate should notify the nsIScriptContext as well, so that it can set aside its termination function chain like it does at all other evaluation entrances.

Requesting blocking, because this can cause serious security bugs (e.g. bug 466826) to be much more random, or even be temporarily hidden in Firefox (which is what we run regression tests on) but not in other Gecko apps.

Comment 1

[Johnny Stenback (:jst)]

Not blocking on this, but marking security sensitive as it's not clear whether this causes security problems or not.

Comment 2

[Boris Zbarsky [:bzbarsky]]

It can cause Gecko bugs bugs to appear or disappear based on what the UI is doing, which just seems like a bad idea.

I don't think it's security-sensitive per se, but I _do_ think it effectively decreases our mochitest test coverage.  It's impossible to write a reliable test for bug 466826, for example, due to this bug.

Comment 3

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.