Last Comment Bug 646662 - ###!!! ASSERTION: This is unsafe! Fix the caller! in nsEventDispatcher
: ###!!! ASSERTION: This is unsafe! Fix the caller! in nsEventDispatcher
Status: VERIFIED FIXED
[sg:critical?] fixed on trunk by 6504...
: verified1.9.2
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: unspecified
: x86 Windows 7
: -- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 650493
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-30 15:44 PDT by Nils
Modified: 2011-08-10 10:50 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
+
fixed
-
wanted
.18+
.18-fixed


Attachments
testcase (crashes browser) (4.27 KB, text/html)
2011-03-30 15:44 PDT, Nils
no flags Details
gdb output on mac (2.88 KB, text/plain)
2011-03-30 15:45 PDT, Nils
no flags Details
stack during assertion (9.43 KB, text/plain)
2011-03-30 15:46 PDT, Nils
no flags Details
Fixes the assertion (1.25 KB, patch)
2011-05-11 02:11 PDT, Olli Pettay [:smaug]
jonas: review+
jst: approval‑mozilla‑beta+
Details | Diff | Splinter Review
for 1.9.2 (1.39 KB, patch)
2011-06-03 11:15 PDT, Olli Pettay [:smaug]
jonas: review+
dveditz: approval1.9.2.18+
Details | Diff | Splinter Review

Description Nils 2011-03-30 15:44:05 PDT
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
Comment 1 Nils 2011-03-30 15:44:52 PDT
Created attachment 523139 [details]
testcase (crashes browser)
Comment 2 Nils 2011-03-30 15:45:58 PDT
Created attachment 523140 [details]
gdb output on mac
Comment 3 Nils 2011-03-30 15:46:16 PDT
Created attachment 523141 [details]
stack during assertion
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 18:12:24 PDT
Should we be popping removable script blockers before calling userdata handlers?

Can we just drop support for userdata?  :(
Comment 5 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-03-30 19:57:44 PDT
I say the latter!
Comment 6 Olli Pettay [:smaug] 2011-04-06 14:26:51 PDT
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.
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2011-04-28 13:52:10 PDT
Olli, can you provide a patch that fixes the assertion alone and we'll start there?
Comment 8 Asa Dotzler [:asa] 2011-05-10 14:38:43 PDT
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?
Comment 9 Olli Pettay [:smaug] 2011-05-10 23:05:25 PDT
ETA end of this week
Comment 10 Olli Pettay [:smaug] 2011-05-11 01:10:15 PDT
The assertion doesn't occur on trunk anymore.
Possibly fixed in Bug 650493.
Comment 11 Olli Pettay [:smaug] 2011-05-11 01:11:46 PDT
building aurora for testing....
Comment 12 Olli Pettay [:smaug] 2011-05-11 01:41:24 PDT
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.
Comment 13 Olli Pettay [:smaug] 2011-05-11 01:46:50 PDT
Ok, I can still reproduce the assertion on aurora
Comment 14 Olli Pettay [:smaug] 2011-05-11 02:11:20 PDT
Created attachment 531567 [details] [diff] [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 15 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-05-11 15:38:29 PDT
Comment on attachment 531567 [details] [diff] [review]
Fixes the assertion

I suspect fixing bug 639648 will fix the crash.
Comment 16 Asa Dotzler [:asa] 2011-05-18 13:59:14 PDT
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?
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2011-05-18 14:02:20 PDT
Did the patch for bug 639648 fix the crash on fx5?
Comment 18 Asa Dotzler [:asa] 2011-05-19 11:31:23 PDT
Boris, yes, that fix seems to have stopped the crash on Aurora.  Do we still need this assertion fix?
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2011-05-19 11:35:25 PDT
Not sure.  Check with Jonas?
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2011-05-19 17:19:11 PDT
Comment on attachment 531567 [details] [diff] [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.
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2011-05-26 13:35:17 PDT
This is fixed since bug 650493 landed. We just need to land this on the beta branch. Olli, can you do the landing?
Comment 22 Olli Pettay [:smaug] 2011-05-26 13:37:32 PDT
Yeah, I'll try to do it tomorrow.
(Sorry, I was traveling for few days)
Comment 23 Olli Pettay [:smaug] 2011-05-31 11:48:33 PDT
I'm having trouble to clone mozilla-beta...
Comment 24 Asa Dotzler [:asa] 2011-05-31 18:27:54 PDT
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
Comment 25 Olli Pettay [:smaug] 2011-06-01 02:08:12 PDT
I'll re-try today. the problem must have been just some networking
error.
Comment 27 Daniel Veditz [:dveditz] 2011-06-03 10:54:28 PDT
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
Comment 28 Olli Pettay [:smaug] 2011-06-03 11:13:55 PDT
I can't reproduce the problem on 192, and the patch doesn't apply cleanly.

But I'll update the patch for 192 anyway.
Comment 29 Olli Pettay [:smaug] 2011-06-03 11:15:20 PDT
Created attachment 537187 [details] [diff] [review]
for 1.9.2

But to fix the real problem, we need Bug 639648
Comment 30 christian 2011-06-06 10:16:09 PDT
We have Bug 639648 on 1.9.2...does that mean we don't need this? Or do they both need to come in?
Comment 31 Olli Pettay [:smaug] 2011-06-06 14:50:10 PDT
I think we should take this, but this needs r and a.
Comment 32 Daniel Veditz [:dveditz] 2011-06-08 10:43:24 PDT
Comment on attachment 537187 [details] [diff] [review]
for 1.9.2

Approved for 1.9.2.18, a=dveditz for release-drivers

Please land after getting jonas's OK.
Comment 34 Al Billings [:abillings] 2011-06-09 11:08:16 PDT
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?
Comment 35 Trif Andrei-Alin[:AlinT] 2011-08-10 08:32:28 PDT
Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.20) Gecko/20110803 Firefox/3.6.20
Runned the testcase and the browser did not crash.
Setting resolution to VERIFIED FIXED.
Thanks!

Note You need to log in before you can comment on or make changes to this bug.