Note: There are a few cases of duplicates in user autocompletion which are being worked on.

###!!! ASSERTION: This is unsafe! Fix the caller! in nsEventDispatcher

VERIFIED FIXED

Status

()

Core
DOM: Core & HTML
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Nils, Assigned: smaug)

Tracking

({verified1.9.2})

unspecified
x86
Windows 7
verified1.9.2
Points:
---

Firefox Tracking Flags

(firefox5+ fixed, firefox6+ fixed, firefox7+ fixed, blocking2.0 -, status2.0 wanted, blocking1.9.2 .18+, status1.9.2 .18-fixed)

Details

(Whiteboard: [sg:critical?] fixed on trunk by 650493 [qa-examined-192])

Attachments

(5 attachments)

(Reporter)

Description

6 years ago
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
(Reporter)

Comment 1

6 years ago
Created attachment 523139 [details]
testcase (crashes browser)
(Reporter)

Comment 2

6 years ago
Created attachment 523140 [details]
gdb output on mac
(Reporter)

Comment 3

6 years ago
Created attachment 523141 [details]
stack during assertion

Comment 4

6 years ago
Should we be popping removable script blockers before calling userdata handlers?

Can we just drop support for userdata?  :(
Status: UNCONFIRMED → NEW
Ever confirmed: true
I say the latter!
(Assignee)

Updated

6 years ago
Component: DOM: Events → DOM: Core & HTML
QA Contact: events → general
Assignee: nobody → Olli.Pettay
status2.0: --- → wanted
Whiteboard: [sg:critical?]
(Assignee)

Comment 6

6 years ago
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?
tracking-firefox5: --- → +
Whiteboard: [sg:critical?] → [sg:critical?][needs response to c7 from Olli]
blocking1.9.2: --- → ?
blocking2.0: --- → -
status1.9.2: --- → wanted
status-firefox5: --- → affected

Comment 8

6 years ago
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?
(Assignee)

Comment 9

6 years ago
ETA end of this week
(Assignee)

Comment 10

6 years ago
The assertion doesn't occur on trunk anymore.
Possibly fixed in Bug 650493.
(Assignee)

Comment 11

6 years ago
building aurora for testing....
(Assignee)

Comment 12

6 years ago
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.
(Assignee)

Comment 13

6 years ago
Ok, I can still reproduce the assertion on aurora
(Assignee)

Comment 14

6 years ago
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.
Attachment #531567 - Flags: review?(jonas)
Comment on attachment 531567 [details] [diff] [review]
Fixes the assertion

I suspect fixing bug 639648 will fix the crash.
Attachment #531567 - Flags: review?(jonas) → review+
(Assignee)

Updated

6 years ago
Attachment #531567 - Flags: approval-mozilla-aurora?
blocking1.9.2: ? → .18+
Depends on: 650493
Whiteboard: [sg:critical?][needs response to c7 from Olli] → [sg:critical?][needs aurora approval] fixed on trunk by 650493?
Attachment #531567 - Flags: approval-mozilla-beta?

Comment 16

6 years ago
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

6 years ago
Did the patch for bug 639648 fix the crash on fx5?

Updated

6 years ago
Attachment #531567 - Flags: approval-mozilla-aurora?

Comment 18

6 years ago
Boris, yes, that fix seems to have stopped the crash on Aurora.  Do we still need this assertion fix?

Comment 19

6 years ago
Not sure.  Check with Jonas?
Whiteboard: [sg:critical?][needs aurora approval] fixed on trunk by 650493? → [sg:critical?][needs beta approval] fixed on trunk by 650493
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.
Attachment #531567 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [sg:critical?][needs beta approval] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493
This is fixed since bug 650493 landed. We just need to land this on the beta branch. Olli, can you do the landing?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
status-firefox6: --- → fixed
status-firefox7: --- → fixed
tracking-firefox6: --- → +
tracking-firefox7: --- → +
Resolution: --- → FIXED
(Assignee)

Comment 22

6 years ago
Yeah, I'll try to do it tomorrow.
(Sorry, I was traveling for few days)
(Assignee)

Comment 23

6 years ago
I'm having trouble to clone mozilla-beta...
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Updated

6 years ago
Whiteboard: [sg:critical?] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493 [Ollie needs help landing for Firefox 5]

Comment 24

6 years ago
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
(Assignee)

Comment 25

6 years ago
I'll re-try today. the problem must have been just some networking
error.
(Assignee)

Comment 26

6 years ago
http://hg.mozilla.org/releases/mozilla-beta/rev/aff078c52e46
status-firefox5: affected → fixed
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
Keywords: checkin-needed
(Assignee)

Comment 28

6 years ago
I can't reproduce the problem on 192, and the patch doesn't apply cleanly.

But I'll update the patch for 192 anyway.
(Assignee)

Comment 29

6 years ago
Created attachment 537187 [details] [diff] [review]
for 1.9.2

But to fix the real problem, we need Bug 639648
Attachment #537187 - Flags: review?(jonas)
Attachment #537187 - Flags: approval1.9.2.18?
(Assignee)

Updated

6 years ago
Whiteboard: [sg:critical?] fixed on trunk by 650493 [Ollie needs help landing for Firefox 5] → [sg:critical?] fixed on trunk by 650493

Comment 30

6 years ago
We have Bug 639648 on 1.9.2...does that mean we don't need this? Or do they both need to come in?
(Assignee)

Comment 31

6 years ago
I think we should take this, but this needs r and a.
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.
Attachment #537187 - Flags: approval1.9.2.18? → approval1.9.2.18+
Attachment #537187 - Flags: review?(jonas) → review+
(Assignee)

Comment 33

6 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1878344aded9
status1.9.2: wanted → .18-fixed
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?
Whiteboard: [sg:critical?] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493 [qa-examined-192]
Group: core-security
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!
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.