Unsafe DOM mutation events in <textarea>

RESOLVED FIXED

Status

()

Core
DOM: Events
--
critical
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Vlad Sukhoy, Assigned: smaug)

Tracking

(4 keywords)

1.8 Branch
x86
All
crash, fixed1.8.1.5, testcase, verified1.8.0.13
Points:
---
Bug Flags:
blocking1.8.1.5 +
blocking1.8.0.13 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low? null deref?] keep private until 355548 is fixed)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 266725 [details]
test case (crashes 1_8 and 1_8_0).

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

Comment 1

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

Updated

10 years ago
Assignee: nobody → Olli.Pettay
(Assignee)

Comment 2

10 years ago
Created attachment 266754 [details] [diff] [review]
WIP

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

Updated

10 years ago
Status: NEW → ASSIGNED
Component: Layout → DOM: Events
(Assignee)

Updated

10 years ago
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
(Assignee)

Comment 3

10 years ago
Created attachment 266793 [details] [diff] [review]
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.
Attachment #266754 - Attachment is obsolete: true
Attachment #266793 - Flags: superreview?(jonas)
Attachment #266793 - Flags: review?(jonas)
(Reporter)

Comment 4

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

Comment 5

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

Comment 6

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

Updated

10 years ago
Blocks: 382754
Attachment #266793 - Flags: superreview?(jonas)
Attachment #266793 - Flags: superreview+
Attachment #266793 - Flags: review?(jonas)
Attachment #266793 - Flags: review+
(Assignee)

Comment 7

10 years ago
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).
Attachment #266793 - Flags: approval1.8.1.5?
(Reporter)

Comment 8

10 years ago
Why not approval1.8.0.13 as well though?
I'll try to generate another testcase...
(Assignee)

Comment 9

10 years ago
Will there be 1.8.0.13? I thought 1.8.0.12 was the last 1.8.0.x one.
(Reporter)

Comment 10

10 years ago
Indeed. Why do we get those guys in bugzilla then?
There will be a 1.8.0.13 for Thunderbird, but [probably] not for Firefox. The flags exist for those bugs, aiui.
(Reporter)

Comment 12

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

Comment 13

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

Comment 14

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

Updated

10 years ago
Attachment #266793 - Flags: approval1.8.1.5?
(Assignee)

Updated

10 years ago
Depends on: 382681
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
(Assignee)

Comment 15

10 years ago
So the patch in Bug 382681 fixes this too.
(Reporter)

Updated

10 years ago
No longer blocks: 382754
(Assignee)

Comment 16

10 years ago
Fixed by the patch in bug 382681.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: fixed1.8.0.13, fixed1.8.0.5
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Keywords: fixed1.8.0.5 → fixed1.8.1.5
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.
Whiteboard: [sg:low? null deref?]
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.
Keywords: fixed1.8.0.13 → verified1.8.0.13
Flags: in-testsuite?
Whiteboard: [sg:low? null deref?] → [sg:low? null deref?] keep private until 355548 is fixed
Group: core-security

Comment 19

8 years ago
crash test landed
http://hg.mozilla.org/mozilla-central/rev/ae37a2531c4b
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.