Closed Bug 1266963 (CVE-2016-5254) Opened 8 years ago Closed 8 years ago

Heap-use-after-free in nsXULPopupManager::KeyDown

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: inferno, Assigned: enndeakin)

References

Details

(Keywords: csectype-uaf, regression, sec-moderate, Whiteboard: [adv-main48+] btpp-active)

Attachments

(1 file, 1 obsolete file)

I don't have a testcase for this, but this stack keeps coming every now and then. looks to be some combination of testcase and keyboard/mouse gestures. Bug seems pretty clear, in nsXULPopupManager::HidePopupCallback, we call "1131   delete item;" which deletes the item (GetTopVisibleMenu()) but does not null it out or store it as strong pointer. Later this deleted pointer is accessed in nsXULPopupManager::KeyDown.

==29531==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b000b353b0 at pc 0x7fb96b53c66b bp 0x7ffe58230850 sp 0x7ffe58230848
READ of size 4 at 0x60b000b353b0 thread T0
    #0 0x7fb96b53c66a in IgnoreKeys layout/xul/nsXULPopupManager.h:167:38
    #1 0x7fb96b53c66a in nsXULPopupManager::KeyDown(nsIDOMKeyEvent*) layout/xul/nsXULPopupManager.cpp:2608
    #2 0x7fb96b53b8f5 in nsXULPopupManager::HandleEvent(nsIDOMEvent*) layout/xul/nsXULPopupManager.cpp:2521:12
    #3 0x7fb968d8fa69 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) dom/events/EventListenerManager.cpp:1099:16
    #4 0x7fb968d913d0 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) dom/events/EventListenerManager.cpp:1270:17
    #5 0x7fb968d6a34d in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:347:7
    #6 0x7fb968d6eedc in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) dom/events/EventDispatcher.cpp:700:9
    #7 0x7fb96af3f6d7 in ForwardKeyToInputMethodAppOrDispatch layout/base/nsPresShell.cpp:7228:3
    #8 0x7fb96af3f6d7 in PresShell::HandleKeyboardEvent(nsINode*, mozilla::WidgetKeyboardEvent&, bool, nsEventStatus*, mozilla::EventDispatchingCallback*) layout/base/nsPresShell.cpp:7129
    #9 0x7fb96af4e9ac in PresShell::DispatchEventToDOM(mozilla::WidgetEvent*, nsEventStatus*, nsPresShellEventCB*) layout/base/nsPresShell.cpp:8324:7
    #10 0x7fb96af49dd1 in PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) layout/base/nsPresShell.cpp:8204:11
    #11 0x7fb96af442d9 in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) layout/base/nsPresShell.cpp:7913:12
    #12 0x7fb96a4de519 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) view/nsViewManager.cpp:798:7
    #13 0x7fb96a4d6310 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) view/nsView.cpp:1140:5
    #14 0x7fb96a588118 in nsWindow::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) widget/gtk/nsWindow.cpp:565:17
    #15 0x7fb96a4e7b8f in nsBaseWidget::ProcessUntransformedAPZEvent(mozilla::WidgetInputEvent*, mozilla::layers::ScrollableLayerGuid const&, unsigned long, nsEventStatus) widget/nsBaseWidget.cpp:1069:3
    #16 0x7fb96a4e86b3 in nsBaseWidget::DispatchInputEvent(mozilla::WidgetInputEvent*) widget/nsBaseWidget.cpp:1184:14
    #17 0x7fb96a528c32 in mozilla::widget::TextEventDispatcher::DispatchInputEvent(nsIWidget*, mozilla::WidgetInputEvent&, nsEventStatus&) widget/TextEventDispatcher.cpp:206:15
    #18 0x7fb96a52c54d in mozilla::widget::TextEventDispatcher::DispatchKeyboardEventInternal(mozilla::EventMessage, mozilla::WidgetKeyboardEvent const&, nsEventStatus&, void*, unsigned int) widget/TextEventDispatcher.cpp:522:3
    #19 0x7fb96a5a0f11 in nsWindow::DispatchKeyDownEvent(_GdkEventKey*, bool*) widget/gtk/nsWindow.cpp:2987:9
    #20 0x7fb96a5a1c15 in nsWindow::OnKeyPressEvent(_GdkEventKey*) widget/gtk/nsWindow.cpp:3060:9
    #21 0x7fb96a5ac226 in key_press_event_cb(_GtkWidget*, _GdkEventKey*) widget/gtk/nsWindow.cpp:5886:12
    #22 0x7fb978b8dc4b in libgtk-3.so.0
0x60b000b353b0 is located 16 bytes inside of 40-byte region [0x60b000b353a0,0x60b000b353c8)
freed by thread T0 here:
    #0 0x4b6aa0 in __interceptor_free _asan_rtl_
    #1 0x7fb96b532749 in operator delete objdir-ff-asan/dist/include/mozilla/mozalloc.h:210:12
    #2 0x7fb96b532749 in nsXULPopupManager::HidePopupCallback(nsIContent*, nsMenuPopupFrame*, nsIContent*, nsIContent*, nsPopupType, bool) layout/xul/nsXULPopupManager.cpp:1131
    #3 0x7fb96b5315ba in nsXULPopupManager::FirePopupHidingEvent(nsIContent*, nsIContent*, nsIContent*, nsPresContext*, nsPopupType, bool, bool) layout/xul/nsXULPopupManager.cpp:1562:7
    #4 0x7fb96b524ed5 in nsXULPopupManager::HidePopup(nsIContent*, bool, bool, bool, bool, nsIContent*) layout/xul/nsXULPopupManager.cpp:1038:7
    #5 0x7fb96b5241d0 in nsXULPopupManager::Rollup(unsigned int, bool, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits> const*, nsIContent**) layout/xul/nsXULPopupManager.cpp:313:7
    #6 0x7fb96b53c37c in nsXULPopupManager::KeyDown(nsIDOMKeyEvent*) layout/xul/nsXULPopupManager.cpp:2596:11
    #7 0x7fb96b53b8f5 in nsXULPopupManager::HandleEvent(nsIDOMEvent*) layout/xul/nsXULPopupManager.cpp:2521:12
    #8 0x7fb968d8fa69 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) dom/events/EventListenerManager.cpp:1099:16
    #9 0x7fb968d913d0 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) dom/events/EventListenerManager.cpp:1270:17
    #10 0x7fb968d6a34d in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:347:7
    #11 0x7fb968d6eedc in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) dom/events/EventDispatcher.cpp:700:9
    #12 0x7fb96af3f6d7 in ForwardKeyToInputMethodAppOrDispatch layout/base/nsPresShell.cpp:7228:3
    #13 0x7fb96af3f6d7 in PresShell::HandleKeyboardEvent(nsINode*, mozilla::WidgetKeyboardEvent&, bool, nsEventStatus*, mozilla::EventDispatchingCallback*) layout/base/nsPresShell.cpp:7129
    #14 0x7fb96af4e9ac in PresShell::DispatchEventToDOM(mozilla::WidgetEvent*, nsEventStatus*, nsPresShellEventCB*) layout/base/nsPresShell.cpp:8324:7
    #15 0x7fb96af49dd1 in PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) layout/base/nsPresShell.cpp:8204:11
    #16 0x7fb96af442d9 in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) layout/base/nsPresShell.cpp:7913:12
    #17 0x7fb96a4de519 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) view/nsViewManager.cpp:798:7
    #18 0x7fb96a4d6310 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) view/nsView.cpp:1140:5
    #19 0x7fb96a588118 in nsWindow::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) widget/gtk/nsWindow.cpp:565:17
    #20 0x7fb96a4e7b8f in nsBaseWidget::ProcessUntransformedAPZEvent(mozilla::WidgetInputEvent*, mozilla::layers::ScrollableLayerGuid const&, unsigned long, nsEventStatus) widget/nsBaseWidget.cpp:1069:3
    #21 0x7fb96a4e86b3 in nsBaseWidget::DispatchInputEvent(mozilla::WidgetInputEvent*) widget/nsBaseWidget.cpp:1184:14
    #22 0x7fb96a528c32 in mozilla::widget::TextEventDispatcher::DispatchInputEvent(nsIWidget*, mozilla::WidgetInputEvent&, nsEventStatus&) widget/TextEventDispatcher.cpp:206:15
    #23 0x7fb96a52c54d in mozilla::widget::TextEventDispatcher::DispatchKeyboardEventInternal(mozilla::EventMessage, mozilla::WidgetKeyboardEvent const&, nsEventStatus&, void*, unsigned int) widget/TextEventDispatcher.cpp:522:3
    #24 0x7fb96a5a0f11 in nsWindow::DispatchKeyDownEvent(_GdkEventKey*, bool*) widget/gtk/nsWindow.cpp:2987:9
    #25 0x7fb96a5a1c15 in nsWindow::OnKeyPressEvent(_GdkEventKey*) widget/gtk/nsWindow.cpp:3060:9
    #26 0x7fb96a5ac226 in key_press_event_cb(_GtkWidget*, _GdkEventKey*) widget/gtk/nsWindow.cpp:5886:12
    #27 0x7fb978b8dc4b in libgtk-3.so.0

previously allocated by thread T0 here:
    #0 0x4b6d98 in malloc _asan_rtl_
    #1 0x4e87dd in moz_xmalloc memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7fb96b52d52e in operator new objdir-ff-asan/dist/include/mozilla/mozalloc.h:186:12
    #3 0x7fb96b52d52e in nsXULPopupManager::ShowPopupCallback(nsIContent*, nsMenuPopupFrame*, bool, bool) layout/xul/nsXULPopupManager.cpp:877
    #4 0x7fb96b52b631 in nsXULPopupManager::FirePopupShowingEvent(nsIContent*, bool, bool) layout/xul/nsXULPopupManager.cpp:1473:7
    #5 0x7fb96b52c50e in nsXULPopupManager::ShowPopupAtScreen(nsIContent*, int, int, bool, nsIDOMEvent*) layout/xul/nsXULPopupManager.cpp:765:3
    #6 0x7fb96a3945f5 in nsXULPopupListener::LaunchPopup(nsIDOMEvent*, nsIContent*) dom/xul/nsXULPopupListener.cpp:439:5
    #7 0x7fb96a391d95 in nsXULPopupListener::HandleEvent(nsIDOMEvent*) dom/xul/nsXULPopupListener.cpp:215:3
    #8 0x7fb968d8fa69 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) dom/events/EventListenerManager.cpp:1099:16
    #9 0x7fb968d913d0 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) dom/events/EventListenerManager.cpp:1270:17
    #10 0x7fb968d6a831 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:390:9
    #11 0x7fb968d6ac14 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) dom/events/EventDispatcher.cpp:418:5
    #12 0x7fb968d6eedc in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) dom/events/EventDispatcher.cpp:700:9
    #13 0x7fb96af4e9eb in PresShell::DispatchEventToDOM(mozilla::WidgetEvent*, nsEventStatus*, nsPresShellEventCB*) layout/base/nsPresShell.cpp:8327:7
    #14 0x7fb96af49dd1 in PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) layout/base/nsPresShell.cpp:8204:11
    #15 0x7fb96af4b60f in PresShell::HandlePositionedEvent(nsIFrame*, mozilla::WidgetGUIEvent*, nsEventStatus*) layout/base/nsPresShell.cpp:8029:10
    #16 0x7fb96af46d2c in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) layout/base/nsPresShell.cpp:7816:12
    #17 0x7fb96a4de519 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) view/nsViewManager.cpp:798:7
    #18 0x7fb96a4d6310 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) view/nsView.cpp:1140:5
    #19 0x7fb96a588118 in nsWindow::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) widget/gtk/nsWindow.cpp:565:17
    #20 0x7fb96a4e7b8f in nsBaseWidget::ProcessUntransformedAPZEvent(mozilla::WidgetInputEvent*, mozilla::layers::ScrollableLayerGuid const&, unsigned long, nsEventStatus) widget/nsBaseWidget.cpp:1069:3
    #21 0x7fb96a4e86b3 in nsBaseWidget::DispatchInputEvent(mozilla::WidgetInputEvent*) widget/nsBaseWidget.cpp:1184:14
    #22 0x7fb96a59e500 in nsWindow::OnButtonPressEvent(_GdkEventButton*) widget/gtk/nsWindow.cpp:2821:9
    #23 0x7fb96a5ae07c in button_press_event_cb(_GtkWidget*, _GdkEventButton*) widget/gtk/nsWindow.cpp:5633:5
    #24 0x7fb978b8dc4b in libgtk-3.so.0
SUMMARY: AddressSanitizer: heap-use-after-free (/mnt/scratch0/clusterfuzz/slave-bot/builds/linux_asan_firefox/custom/firefox/libxul.so+0x947b66a)
Shadow bytes around the buggy address:
  0x0c168015ea20: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c168015ea30: fa fa fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c168015ea40: fa fa fa fa fa fa fa fa fd fd fd fd fd fa fa fa
  0x0c168015ea50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
  0x0c168015ea60: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c168015ea70: fa fa fa fa fd fd[fd]fd fd fa fa fa fa fa fa fa
  0x0c168015ea80: fa fa fa fa fa fa fa fa fa fa fd fd fd fd fd fa
  0x0c168015ea90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c168015eaa0: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa
  0x0c168015eab0: fa fa fa fa fa fa fd fd fd fd fd fa fa fa fa fa
  0x0c168015eac0: fa fa fa fa fa fa fa fa fa fa fa fa fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==29531==ABORTING
I'm going to mark this sec-high because it sounds intermittent, and I'm not sure how content would really trigger this, but I guess it happens somehow.

Neil, it looks like you've worked on this file before. Could you look into fixing this? Thanks.
Flags: needinfo?(enndeakin)
Is this Linux only? It looks like a variation of 911546.
Flags: needinfo?(enndeakin)
It is hard to say if it is only on Linux, because ASan is mostly Linux-only.
Group: core-security → dom-core-security
Can we just null out the deleted field like comment 0 suggests?
Flags: needinfo?(enndeakin)
Attached patch Move stopPropagation call (obsolete) — Splinter Review
We can just move the code to before it gets deleted.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Attachment #8747260 - Flags: review?(masayuki)
Comment on attachment 8747260 [details] [diff] [review]
Move stopPropagation call

Looks good to me. But for preventing similar bug, could you clear item to nullptr in |if (!(ctrl || alt || shift || meta)) {| with a comment?
Attachment #8747260 - Flags: review?(masayuki) → review+
This isn't a high security issue, but is a crash that can easily be reproduced. It is a regression from bug 1191897.

To trigger it:

1. Open a menu on the toplevel menubar such as the File menu.
2. Press alt

Content cannot trigger this, and we return earlier for non-trusted events in nsXULPopupManager::HandleEvent so it can only be triggered by trusted key events.

Can we change the security flags as needed?
Flags: needinfo?(continuation)
Attachment #8747260 - Attachment is obsolete: true
Alright, maybe sec-moderate makes more sense. Thanks for fixing this!
Flags: needinfo?(continuation)
Keywords: sec-highsec-moderate
Whiteboard: btpp-active
https://hg.mozilla.org/mozilla-central/rev/1d23af51e886
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Group: dom-core-security → core-security-release
Flags: sec-bounty?
Blocks: 1191897
Flags: sec-bounty? → sec-bounty+
Keywords: regression
Would this be good to uplift to 48? Or do you think it's better to leave this to ride along with 49? Since it's a regression from 43 it doesn't seem too urgent to uplift, but if it might mean fewer crashes in 48 with e10s enabled, maybe it's worth it.
Flags: needinfo?(enndeakin)
Comment on attachment 8747721 [details] [diff] [review]
Move stopPropagation call, add a test

Approval Request Comment
[Feature/regressing bug #]: 1266963
[User impact if declined]: easily reproducable crash, but not common as menubars are not visible by default
[Describe test coverage new/current, TreeHerder]: has automated test
[Risks and why]: just moves code around so no risk
[String/UUID change made/needed]: none
Flags: needinfo?(enndeakin)
Attachment #8747721 - Flags: approval-mozilla-aurora?
Attachment #8747721 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi Abhishek, is this still happening for you with a current build of Fx 48?
Flags: needinfo?(inferno)
I don't know about 48, but tip-of-tree trunk build is fixed.
Flags: needinfo?(inferno)
Alias: CVE-2016-5254
Whiteboard: btpp-active → [adv-main48+] btpp-active
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.