Bugzilla@Mozilla – Bug 646662
###!!! ASSERTION: This is unsafe! Fix the caller! in nsEventDispatcher
Last modified: 2011-07-12 08:32:10 PDT
Summon comment box
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0 Build Identifier: Description: The attached testcase triggers following assertion on debug builds: ###!!! ASSERTION: This is unsafe! Fix the caller! A stack back trace at the time of the assertion is attached. Additionally exploitable looking crashes have been observed on Linux, Windows and Mac. The reduced test case needs a few reloads to crash the browser (especially on windows). The likelihood of crashes can be increased by loading the testcase in several tabs and iframes. Regardless of the crashes, the assertion seem to happen for every reload. Affected Versions: Firefox 4.0 Trunk Testcase: The testcase is attached as an HTML file. It will crash the browser on opening after several reloads. Testcase Notes: gc() triggers garbage collection. It requires Jesse's quitter extension (https://www.squarefree.com/extensions/quitter.xpi). Stack Backtrace: The stack during the first assertion on Linux is attached. VulnDev reference : vd11007 reported by nils of vulndev ltd. Reproducible: Always
Created attachment 523139 [details] testcase (crashes browser)
Created attachment 523140 [details] gdb output on mac
Created attachment 523141 [details] stack during assertion
Should we be popping removable script blockers before calling userdata handlers? Can we just drop support for userdata? :(
I say the latter!
Can't reproduce the crash (with quitter.xpi) on 1.9.2 nor with modified testcase (with domFuzzLite.xpi) But the assertion is there, and that is serious enough and fixing the cause for that could hopefully fix the crash too.
Olli, can you provide a patch that fixes the assertion alone and we'll start there?
Ollie, we're coming up to the train station and time is short for a fix that's wanted for Firefox 5. Can you give us an update here, please?
ETA end of this week
The assertion doesn't occur on trunk anymore. Possibly fixed in Bug 650493.
building aurora for testing....
I can't get even the assertion now on 1.9.2 (with https://www.squarefree.com/extensions/quitter.xpi) Will try on aurora once the build is ready.
Ok, I can still reproduce the assertion on aurora
Created attachment 531567 [details] [review] Fixes the assertion As far as I see, we shouldn't have script blocker in stack when calling AdoptNode. On trunk the problem doesn't happen because we use nsUserDataCaller script runner. I doubt the patch helps with the crash (which I can't reproduce) though.
Comment on attachment 531567 [details] [review] Fixes the assertion I suspect fixing bug 639648 will fix the crash.
With the crash fix landed is it still sg:crit to get this assertion fixed? What's the risk of taking this fix so late in the Firefox 5 cycle?
Did the patch for bug 639648 fix the crash on fx5?
Boris, yes, that fix seems to have stopped the crash on Aurora. Do we still need this assertion fix?
Not sure. Check with Jonas?
Comment on attachment 531567 [details] [review] Fixes the assertion Checked with Jonas and he feels more comfortable with taking this than not. This patch prevents us from getting into uncharted territory where exploitable things *could* be possible, though we do not at this point know of any specific ways where an actual exploit would happen.
This is fixed since bug 650493 landed. We just need to land this on the beta branch. Olli, can you do the landing?
Yeah, I'll try to do it tomorrow. (Sorry, I was traveling for few days)
I'm having trouble to clone mozilla-beta...
Ollie, do either of these links help you get your build? http://ftp.mozilla.org/pub/mozilla.org/firefox/bundles/ https://developer.mozilla.org/En/Developer_Guide/Source_Code/Mercurial#Bundles
I'll re-try today. the problem must have been just some networking error.
http://hg.mozilla.org/releases/mozilla-beta/rev/aff078c52e46
Olli: does this patch also for for the 1.9.2 branch or do we need to do something else? code-freeze is Monday June 6
I can't reproduce the problem on 192, and the patch doesn't apply cleanly. But I'll update the patch for 192 anyway.
Created attachment 537187 [details] [review] for 1.9.2 But to fix the real problem, we need Bug 639648
We have Bug 639648 on 1.9.2...does that mean we don't need this? Or do they both need to come in?
I think we should take this, but this needs r and a.
Comment on attachment 537187 [details] [review] for 1.9.2 Approved for 1.9.2.18, a=dveditz for release-drivers Please land after getting jonas's OK.
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1878344aded9
Since there is no STR for 1.9.2 and comment 28 says that the issue cannot be reproduced here on 1.9.2, am I correct in my belief that there is nothing for QA to do here for 1.9.2 verification?