Closed
Bug 1266963
(CVE-2016-5254)
Opened 9 years ago
Closed 9 years ago
Heap-use-after-free in nsXULPopupManager::KeyDown
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: inferno, Assigned: enndeakin)
References
Details
(4 keywords, Whiteboard: [adv-main48+] btpp-active)
Attachments
(1 file, 1 obsolete file)
3.83 KB,
patch
|
abillings
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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
Comment 1•9 years ago
|
||
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)
Keywords: csectype-uaf,
sec-high
Assignee | ||
Comment 2•9 years ago
|
||
Is this Linux only? It looks like a variation of 911546.
Flags: needinfo?(enndeakin)
Comment 3•9 years ago
|
||
It is hard to say if it is only on Linux, because ASan is mostly Linux-only.
Updated•9 years ago
|
Group: core-security → dom-core-security
Updated•9 years ago
|
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 5•9 years ago
|
||
We can just move the code to before it gets deleted.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Assignee | ||
Updated•9 years ago
|
Attachment #8747260 -
Flags: review?(masayuki)
Comment 6•9 years ago
|
||
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+
Assignee | ||
Comment 7•9 years ago
|
||
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)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8747260 -
Attachment is obsolete: true
Comment 9•9 years ago
|
||
Alright, maybe sec-moderate makes more sense. Thanks for fixing this!
Flags: needinfo?(continuation)
Keywords: sec-high → sec-moderate
Updated•9 years ago
|
Whiteboard: btpp-active
Assignee | ||
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d23af51e88680888da7ba6709c04130a977718f
Bug 1266963, stop propagation before other steps, r=masayuki
Comment 11•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•9 years ago
|
Group: dom-core-security → core-security-release
Updated•9 years ago
|
Flags: sec-bounty?
Updated•9 years ago
|
Comment 12•9 years ago
|
||
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)
Assignee | ||
Comment 13•9 years ago
|
||
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?
Updated•9 years ago
|
Attachment #8747721 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 14•9 years ago
|
||
Comment 15•9 years ago
|
||
Hi Abhishek, is this still happening for you with a current build of Fx 48?
Flags: needinfo?(inferno)
Reporter | ||
Comment 16•9 years ago
|
||
I don't know about 48, but tip-of-tree trunk build is fixed.
Flags: needinfo?(inferno)
Updated•9 years ago
|
Alias: CVE-2016-5254
Whiteboard: btpp-active → [adv-main48+] btpp-active
Updated•8 years ago
|
Group: core-security-release
Updated•9 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•