Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Bug 580445 (CVE-2010-2766)

normalizeDocument Remote Code Execution Vulnerability (ZDI-CAN-866)

RESOLVED FIXED

Status

()

Core
DOM
--
critical
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: reed, Assigned: jst)

Tracking

({verified1.9.1, verified1.9.2})

unspecified
verified1.9.1, verified1.9.2
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 final+, status2.0 ?, blocking1.9.2 .9+, status1.9.2 .9-fixed, blocking1.9.1 .12+, status1.9.1 .12-fixed)

Details

(Whiteboard: [sg:critical?])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Created attachment 458836 [details]
PoC

ZDI-CAN-866: Mozilla Firefox normalizeDocument Remote Code Execution Vulnerability

-- CVSS ----------------------------------------------------------------
10, (AV:N/AC:L/Au:N/C:C/I:C/A:C)

-- ABSTRACT ------------------------------------------------------------

TippingPoint has identified a vulnerability affecting the following 
products:

    Mozilla Firefox 3.6.x

-- VULNERABILITY DETAILS -----------------------------------------------

This vulnerability allows remote attackers to execute arbitrary code on
vulnerable installations of Mozilla Firefox. User interaction is
required to exploit this vulnerability in that the target must visit a
malicious page or open a malicious file.

The flaw exists within the normalizeDocument function defined within
nsDocument.cpp. When handling children nodes the code does not account
for a varying number of children during normalization. An attacker can
abuse this problem along with the fact that the code does not validate
the child index is within bounds to access an invalid object and execute
arbitrary code under the context of the browser.

From content/base/src/nsDocument.cpp:

nsDocument::Normalize()
{
PRInt32 count = mChildren.ChildCount();
for (PRInt32 i = 0; i < count; ++i) {
nsCOMPtr<nsIDOMNode> node(do_QueryInterface(mChildren.ChildAt(i)));

if (node) {
node->Normalize();
}
}

return NS_OK;
}

There are two problems in the code above:

1. Number of root's children can change during normalization. Thus
fetching
mChildren.ChildCount() at the very beginning is incorrect.
2. mChildren.ChildAt() is not a fail-safe method, i.e. it does not check
if index argument is within children array bounds.

Attacker can create HTML document with some additional nodes after HTML
body and then remove them during initial steps of normalization process.
This directly leads to arbitrary code execution.

-- CREDIT --------------------------------------------------------------

This vulnerability was discovered by:
    * regenrecht
(Reporter)

Comment 1

7 years ago
bp-e842aa05-915a-4161-abd0-e7e7d2100720

Mozilla/5.0 (X11; Linux i686; rv:2.0b2pre) Gecko/20100720 Minefield/4.0b2pre
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status1.9.1: --- → ?
status1.9.2: --- → ?
status2.0: --- → ?
(Assignee)

Comment 2

7 years ago
Created attachment 458839 [details] [diff] [review]
Fix.
Attachment #458839 - Flags: review?(jonas)
(Assignee)

Comment 3

7 years ago
And "node" in that patch doesn't necessarily need to be an nsCOMPtr, but I think it's safer to leave it be a strong reference in case code down the call chain can trigger destruction of the node or what not.
Comment on attachment 458839 [details] [diff] [review]
Fix.

stupid mutation events
Attachment #458839 - Flags: review?(jonas) → review+

Comment 5

7 years ago
Please add a comment so we don't try to "optimize" this later and reintroduce the bug.
(Assignee)

Comment 6

7 years ago
Fixed.

http://hg.mozilla.org/mozilla-central/rev/bb303ad2db3e
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

7 years ago
Comment on attachment 458839 [details] [diff] [review]
Fix.

This applies cleanly to 1.9.1 and 1.9.2.
Attachment #458839 - Flags: approval1.9.2.8?
Attachment #458839 - Flags: approval1.9.1.12?
(Reporter)

Comment 8

7 years ago
(In reply to comment #5)
> Please add a comment so we don't try to "optimize" this later and reintroduce
> the bug.

Poke. A comment did not get added on commit...
(Assignee)

Comment 9

7 years ago
Duh, totally missed that here. Jesse, do you want me to add a comment now and make it more obvious what we're fixing, or wait until we've released 3.6 etc with this and add a comment to trunk then?

Comment 10

7 years ago
Add a comment when you add the test, I guess.

Btw, the algorithm is probably still "incorrect" in that it can skip children if earlier children are removed in mutation event handlers.  Switching from array indexing to following next-sibling pointers would help, and might improve performance.

Comment 11

7 years ago
How would that help?

The only sane algorithm that can work for a case like this is to build a list of nodes to normalize up front, then normalize them all.  Chasing pointers, for example, can trivially go into an infinite loop if scripts that fire on normalize rearrange the DOM right.
(Reporter)

Updated

7 years ago
Assignee: nobody → jst
(Reporter)

Updated

7 years ago
Flags: in-testsuite?
(Assignee)

Comment 12

7 years ago
Yes, one could argue that the algorithm is "incorrect" but I'm arguing that it's not worth making it "correct" here. There's nothing unsafe about the fixed version of this code that I can see, and if script in a document mutates the document during normalization then, well, it won't be normalized, and I say that's ok.
blocking1.9.1: ? → .12+
blocking1.9.2: ? → .8+
status1.9.1: ? → wanted
status1.9.2: ? → wanted
Comment on attachment 458839 [details] [diff] [review]
Fix.

Approved for 1.9.2.8 and 1.9.1.12, a=dveditz for release-drivers
Attachment #458839 - Flags: approval1.9.2.8?
Attachment #458839 - Flags: approval1.9.2.8+
Attachment #458839 - Flags: approval1.9.1.12?
Attachment #458839 - Flags: approval1.9.1.12+
(Assignee)

Comment 14

7 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5da73af750ce
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c1d0170d5045
status1.9.1: wanted → .12-fixed
status1.9.2: wanted → .9-fixed
Verified for 1.9.2.9 with PoC and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9pre) Gecko/20100809 Namoroka/3.6.9pre ( .NET CLR 3.5.30729). PoC crashes 1.9.2.8.

Verified for 1.9.1.12 with PoC and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.12pre) Gecko/20100809 Shiretoko/3.5.12pre ( .NET CLR 3.5.30729). Crashes 1.9.1.11.
Keywords: verified1.9.1, verified1.9.2
Alias: CVE-2010-2766
blocking2.0: ? → final+
Group: core-security
You need to log in before you can comment on or make changes to this bug.