382568 - Unsafe DOM mutation events in <textarea>

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Vlad Sukhoy]

Attachment: test case (crashes 1_8 and 1_8_0).

DOMAttrModified events are fired from the presentation code which triggers JavaScript code within unsafe context.

Comment 1

[Vlad Sukhoy]

See bug 355548 for the discussion on related issue.

In this particular case, nsTextControlFrame::CreateAnonymousContent does attribute manipulation on the content <div> and this fires DOMAttrModified event, which in this context is unsafe.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: WIP

Will do some tests, and possibly add a method to nsGenericHTMLFormElement to 
test whether mutation event is coming from native anon.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch

Handling mutation events the same way in input and textarea.
<select> has also some native anon content, but couldn't get it to crash
the same way as <textarea>. (I did find one crash, but that is null pointer 
thingie).

On trunk mutation events from native anon are always prevented, but that
relies on the new event dispatching code.

Comment 4

[Vlad Sukhoy]

I'm working on a related testcase to crash generic scrollable frame code (the same stuff in nsGfxScrollFrameInner::CreateAnonymousContent()). 
The whole thing of branches firing mutation whenever a frame changes something in content is vulnerable IMO.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #4)
> The whole thing of branches firing mutation whenever a frame changes something
> in content is vulnerable IMO.
> 

Yes, it is, though in trunk there shouldn't be any problems if content is native
anonymous. And note, nsGfxScrollFrame(Inner) has changed quite a bit since 1.8.
If you find a new crash, IsEventHandlingDisabled could be moved up to 
nsGenericElement.

Comment 6

[Vlad Sukhoy]

Could not get it to crash in this particular case, but definitely was able to trash the UI thread with stuff, see bug 382681. 

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 266793 [details] [diff] [review]
proposed patch

I'd take this to 1.8 and if someone can write a testcase where mutation events coming from native anonymous are causing crashes, ::IsEventHandlingDisabled() could be pushed up to nsGenericElement, (or a new patch with similar functionality could be written).

Comment 8

[Vlad Sukhoy]

Why not approval1.8.0.13 as well though?
I'll try to generate another testcase...

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Will there be 1.8.0.13? I thought 1.8.0.12 was the last 1.8.0.x one.

Comment 10

[Vlad Sukhoy]

Indeed. Why do we get those guys in bugzilla then?

Comment 11

[Samuel Sidler (old account; do not CC)]

There will be a 1.8.0.13 for Thunderbird, but [probably] not for Firefox. The flags exist for those bugs, aiui.

Comment 12

[Vlad Sukhoy]

Ok, then the flag should be probably up, and also for those guys who might still use 1.8.0 layout in their code..

Comment 13

[Vlad Sukhoy]

@Smaug: I finally was able to crash scrollable frame code, see attachment 267625 [details]. Is that enough to warrant promotion of attachment 266793 [details] [diff] [review] to nsGenericElement?

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Because of XUL's strange way to flag native anonymous content, that 
may not work after all. I'll do some tests and write a bit different
kind of patch if needed.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

So the patch in Bug 382681 fixes this too.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Fixed by the patch in bug 382681.

Comment 17

[Daniel Veditz [:dveditz]]

In FF2.0.0.4 the crash I get is a null deref. This testcase doesn't lead to an exploitable situation but that doesn't mean these unsafe events can't be abused in other ways.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

I can verify that we don't crash using Thunderbird version 1.5.0.13 (20070809) (Mac OS X) using the testcase in comment 0; replacing fixed1.5.0.13 with verified1.5.0.13.

Comment 19

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/ae37a2531c4b