Closed Bug 787704 (CVE-2012-3990) Opened 12 years ago Closed 12 years ago

use-after-free in nsIContent::GetNameSpaceID

Categories

(Core :: XUL, defect)

Product:
Core
Component:
XUL
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18
Tracking Flags:
Tracking Status
firefox16 + fixed
firefox17 + fixed
firefox18 + fixed
firefox-esr10 16+ fixed

People

(Reporter: miaubiz, Assigned: smaug)

Details

(5 keywords, Whiteboard: [asan][advisory-tracking+])

Attachments

(5 files)

repro case
1.20 KB, text/html
Details
asan log linux nightly
6.29 KB, text/plain
Details
asan log linux aurora
6.29 KB, text/plain
Details
patch
1.96 KB, patch
masayuki
: review+
akeybl
: approval-mozilla-aurora+
Details | Diff | Splinter Review
for branches with nsnull
1.96 KB, patch
akeybl
: approval-mozilla-beta+
akeybl
: approval-mozilla-esr10+
Details | Diff | Splinter Review
Attached file repro case —
when I load: <html> <head> <style> .c0 { content:'A' } </style> <script> onload = function() { document.body.appendChild(document.createElement('td')) el0=document.createElement('div') document.body.appendChild(el0) el1=document.createElement('form') document.body.appendChild(el1) el2=document.createElement('input') el2.setAttribute('type', 'submit') el1.appendChild(el2) el3=document.createElement('input') el3.setAttribute('required', 'A') el1.appendChild(el3) el4=document.createElement('input') el4.type='file' el1.appendChild(el4) setTimeout(function() { el4.focus() setTimeout(function() { el4.click() el2.setAttribute('required', 'A') el0.setAttribute('class', 'c0') setTimeout(function() { document.designMode='on' document.execCommand('italic') document.designMode='off' setTimeout(function() { el2.click() }, 100) }, 100) }, 100) }, 100) } </script> </head> <body> </body> </html> I get a crash like so: ==24180== ERROR: AddressSanitizer heap-use-after-free on address 0x7fffbe11ac98 at pc 0x7fffeeb97d74 bp 0x7ffffffedd70 sp 0x7ffffffedd68 READ of size 8 at 0x7fffbe11ac98 thread T0 #0 0x7fffeeb97d74 in nsIContent::GetNameSpaceID() const /builds/slave/try-lnx64/build/../../../dist/include/nsINodeInfo.h:143 #1 0x7fffee6f9a37 in nsCOMPtr<nsIDOMEventTarget>::operator=(nsCOMPtr<nsIDOMEventTarget> const&) /builds/slave/try-lnx64/build/../../../../dist/include/nsCOMPtr.h:614 #2 0x7fffee6fe5fb in nsXULPopupManager::ShowPopupCallback(nsIContent*, nsMenuPopupFrame*, bool, bool) /builds/slave/try-lnx64/build/layout/xul/base/src/nsXULPopupManager.cpp:716 #3 0x7fffee6fc7a2 in nsXULPopupManager::FirePopupShowingEvent(nsIContent*, bool, bool) /builds/slave/try-lnx64/build/layout/xul/base/src/nsXULPopupManager.cpp:1181 #4 0x7fffee6fcf36 in nsXULPopupManager::ShowPopup(nsIContent*, nsIContent*, nsAString_internal const&, int, int, bool, bool, bool, nsIDOMEvent*) /builds/slave/try-lnx64/build/layout/xul/base/src/nsXULPopupManager.cpp:568 #5 0x7fffee691040 in non-virtual thunk to nsPopupBoxObject::OpenPopup(nsIDOMElement*, nsAString_internal const&, int, int, bool, bool, nsIDOMEvent*) /builds/slave/try-lnx64/build/../../../../dist/include/nsCOMPtr.h:407 0x7fffbe11ac98 is located 24 bytes inside of 368-byte region [0x7fffbe11ac80,0x7fffbe11adf0) freed by thread T0 here: #0 0x42ae21 in free ??:0 #1 0x7fffee92a335 in nsNodeUtils::LastRelease(nsINode*) /builds/slave/try-lnx64/build/content/base/src/nsNodeUtils.cpp:261 #2 0x7fffee9ff963 in mozilla::dom::FragmentOrElement::Release() /builds/slave/try-lnx64/build/content/base/src/FragmentOrElement.cpp:1853 #3 0x7fffef02ebc4 in nsGlobalWindow::SetFocusedNode(nsIContent*, unsigned int, bool) /builds/slave/try-lnx64/build/dom/base/nsGlobalWindow.cpp:7765 #4 0x7fffeefc93be in nsFocusManager::SetFocusInner(nsIContent*, int, bool, bool) aurora and nightly affected atleast.
Attached file asan log linux nightly —
Attached file asan log linux aurora —
Component: General → XP Toolkit/Widgets: Menus
Product: Firefox → Core
Attachment #657590 - Attachment mime type: text/plain → text/html
We end up having sContent pointing to a deleted object. sPresContext looks scary too, and sTextStateObserver. Masayuki, could you look at this? If not, I will.
Hmm, I guess sPresContext is ok and sTextStateObserver is actually refcounted.
Actually, I could take this.
Assignee: nobody → bugs
Or no :) My initial approach doesn't seem to work.
Assignee: bugs → nobody
Assignee: nobody → bugs
Attached patch patch — — Splinter Review
Attachment #657613 - Flags: review?(masayuki)
Comment on attachment 657613 [details] [diff] [review] patch IIRC static nsCOMPtrs should be avoided, so I used manual refcounting here. Also, since sContent is released when prescontext goes away, there shouldn't be leaks.
Severity: normal → critical
Keywords: crash, sec-critical, testcase
OS: Linux → All
Hardware: x86 → All
Whiteboard: [asan]
Comment on attachment 657613 [details] [diff] [review] patch This patch looks okay for me. However, I don't understand why does the testcase in comment 0 cause such case. Looks like el4.focus() caused releasing the content, why?
Attachment #657613 - Flags: review?(masayuki) → review+
It goes something like: Focus would be in native anon button of input type file when designMode = on reframes the element.
Attached patch for branches with nsnull — — Splinter Review
(In reply to Olli Pettay [:smaug] from comment #10) > It goes something like: Focus would be in native anon button of input type > file when designMode = on reframes the element. When it's reframed, all of the anon contents will be recreated?
Yes.
Thanks, I understand.
Keywords: csec-uaf
Olli, is this ready to be landed?
(In reply to Andrew McCreight [:mccr8] from comment #16) > Olli, is this ready to be landed? Yes, but wasn't sure whether I should land it immediately.
I'll try to remember to land it tomorrow.
(In reply to Olli Pettay [:smaug] from comment #18) > I'll try to remember to land it tomorrow. Now's the time. Please land and nominate for uplift.
Comment on attachment 657793 [details] [diff] [review] for branches with nsnull [Approval Request Comment] Bug caused by (feature/regressing bug #): NA User impact if declined: Security sensitive crash Testing completed (on m-c, etc.): tested locally. Will land m-c real soon Risk to taking this patch (and alternatives if risky): May keep an element longer than before, so that could increase memory usage temporarily. String or UUID changes made by this patch: NA
Attachment #657793 - Flags: approval-mozilla-esr10?
Attachment #657793 - Flags: approval-mozilla-beta?
Comment on attachment 657613 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): NA User impact if declined: Security sensitive crash Testing completed (on m-c, etc.): tested locally. Will land m-c real soon Risk to taking this patch (and alternatives if risky): May keep an element longer than before, so that could increase memory usage temporarily. String or UUID changes made by this patch: NA
Attachment #657613 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/f8d30ea0974c
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 657613 [details] [diff] [review] patch [Triage Comment] Approving for Aurora in preparation for Beta consideration.
Attachment #657613 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
We'll approve on Monday if there haven't been any Nightly/Aurora regressions (please land on Aurora asap).
Target Milestone: --- → mozilla18
Thanks for landing to Aurora. (I was going to do it later today.)
Comment on attachment 657793 [details] [diff] [review] for branches with nsnull [Triage Comment] Approving for Beta (please land no later than tomorrow morning PT) and ESR10.
Attachment #657793 - Flags: approval-mozilla-esr10?
Attachment #657793 - Flags: approval-mozilla-esr10+
Attachment #657793 - Flags: approval-mozilla-beta?
Attachment #657793 - Flags: approval-mozilla-beta+
Whiteboard: [asan] → [asan][advisory-tracking+]
Alias: CVE-2012-3990
Keywords: verifyme
Group: core-security
Flags: sec-bounty+
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Component: XP Toolkit/Widgets: Menus → XUL
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: