Closed Bug 1317670 Opened 8 years ago Closed 8 years ago

ref_fuzz5 crashes Nightly in mozilla::MediaManager::OnNavigation

Categories

(Core :: WebRTC, defect, P1)

Product:
Core
Component:
WebRTC
Version:
51 Branch
Platform:
x86_64
Windows 8.1
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 + fixed
firefox52 + fixed
firefox53 + fixed

People

(Reporter: geeknik, Assigned: mchiang)

References

(
URL
)

Details

(4 keywords, Whiteboard: [post-critsmash-triage])

Attachments

(2 files, 1 obsolete file)

imported patch
2.44 KB, patch
jib
: review+
dveditz
: sec-approval+
Details | Diff | Splinter Review
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch
1.77 KB, patch
mchiang
: review+
gchang
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
abillings
: sec-approval+
Details | Diff | Splinter Review 
While fuzzing Nightly with ref_fuzz5, this crash happened.
https://crash-stats.mozilla.com/report/index/378396d7-43d6-4774-b4a2-087ae2161115

windbg + a516c754042c438a5c1499171ca525a980ecb911:
(3688.3fc4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
xul!mozilla::MediaManager::OnNavigation+0x1de:
00007ffe`d2707856 4c3998c8000000  cmp     qword ptr [rax+0C8h],r11 ds:e5e5e5e5`e5e5e6ad=????????????????
2:031> ~* kp

. 31  Id: 3688.3fc4 Suspend: 1 Teb: 00007ff6`3380d000 Unfrozen
Child-SP          RetAddr           Call Site
0000003c`cefce510 00007ffe`d216dabc xul!mozilla::MediaManager::OnNavigation(unsigned int64 aWindowID = 0x8000006e)+0x1de [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\media\mediamanager.cpp @ 2698]
0000003c`cefce560 00007ffe`d194366a xul!mozilla::dom::Navigator::OnNavigation(void)+0x28 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\base\navigator.cpp @ 1778]
0000003c`cefce590 00007ffe`d14d8453 xul!nsGlobalWindow::SetNewDocument+0x8bf5be [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\base\nsglobalwindow.cpp @ 2700]
0000003c`cefce8b0 00007ffe`d14d6b9e xul!nsDocumentViewer::InitInternal(class nsIWidget * aParentWidget = 0x0000003c`d92ed000, class nsISupports * aState = 0x00000000`00000000, struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> * aBounds = 0x0000003c`dde40c20, bool aDoCreation = true, bool aNeedMakeCX = true, bool aForceSetNewDocument = true)+0x25b [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\layout\base\nsdocumentviewer.cpp @ 900]
0000003c`cefce950 00007ffe`d131cac5 xul!nsDocumentViewer::Init(class nsIWidget * aParentWidget = 0x0000003c`d6042ec0, struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> * aBounds = 0x0000003c`cefceaa0)+0x1e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\layout\base\nsdocumentviewer.cpp @ 644]
0000003c`cefce9a0 00007ffe`d10789df xul!nsDocShell::SetupNewViewer(class nsIContentViewer * aNewViewer = 0x0000003c`d8f04a60)+0x355 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 9382]
0000003c`cefceb60 00007ffe`d14b69f4 xul!nsDocShell::Embed(class nsIContentViewer * aContentViewer = 0x00000000`80000000, char * aCommand = 0x0000003c`d9043800 "???", class nsISupports * aExtraInfo = 0x00007ffe`d1721562)+0x2f [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 7241]
0000003c`cefceb90 00007ffe`d14b6658 xul!nsDocShell::CreateContentViewer(class nsACString_internal * aContentType = 0x00000000`00000000, class nsIRequest * aRequest = 0x0000003c`dde05dd0, class nsIStreamListener ** aContentHandler = 0x0000003c`dde05dd0)+0x158 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 9187]
0000003c`cefcec40 00007ffe`d14b641a xul!nsDSURIContentListener::DoContent(class nsACString_internal * aContentType = 0x0000003c`d89478b8, bool aIsContentPreferred = true, class nsIRequest * aRequest = 0x00000000`00000000, class nsIStreamListener ** aContentHandler = 0x0000003c`00790800, bool * aAbortProcess = 0x0000003c`cefced98)+0xe8 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdsuricontentlistener.cpp @ 128]
0000003c`cefcecb0 00007ffe`d14b54d5 xul!nsDocumentOpenInfo::TryContentListener(class nsIURIContentListener * aListener = 0x00000000`00000001, class nsIChannel * aChannel = 0x0000003c`e7000620)+0x2fe [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 742]
0000003c`cefced80 00007ffe`d11a847b xul!nsDocumentOpenInfo::DispatchContent(class nsIRequest * request = 0x00000000`00000000, class nsISupports * aCtxt = 0x0000003c`d62f7301)+0x34d [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 414]
0000003c`cefcefa0 00007ffe`d11a7b7f xul!nsDocumentOpenInfo::OnStartRequest(class nsIRequest * request = 0x0000003c`d0f49918, class nsISupports * aCtxt = 0x0000003c`cefcf128)+0x353 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 277]
0000003c`cefcf0c0 00007ffe`d11cbf83 xul!nsBaseChannel::OnStartRequest(class nsIRequest * request = 0x0000003c`db542d70, class nsISupports * ctxt = 0x0000003c`db542d70)+0x7f [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsbasechannel.cpp @ 809]
0000003c`cefcf0f0 00007ffe`d11ccc71 xul!nsInputStreamPump::OnStateStart(void)+0xc3 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsinputstreampump.cpp @ 525]
0000003c`cefcf120 00007ffe`d11ccb64 xul!nsInputStreamPump::OnInputStreamReady(class nsIAsyncInputStream * stream = 0x0000003c`d6047d68)+0xf9 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsinputstreampump.cpp @ 441]
0000003c`cefcf160 00007ffe`d120a83f xul!nsOutputStreamReadyEvent::Run(void)+0x24 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\xpcom\io\nsstreamutils.cpp @ 97]
0000003c`cefcf190 00007ffe`d12087c5 xul!nsThread::ProcessNextEvent(bool aMayWait = true, bool * aResult = 0x0000003c`cefcf5f0)+0x4b3 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\xpcom\threads\nsthread.cpp @ 1216]
0000003c`cefcf2a0 00007ffe`d1d3f600 xul!mozilla::ipc::MessagePump::Run(class base::MessagePump::Delegate * aDelegate = 0x0000003c`cefcf601)+0x9d [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\glue\messagepump.cpp @ 96]
0000003c`cefcf320 00007ffe`d15abb8f xul!mozilla::ipc::MessagePumpForChildProcess::Run(class base::MessagePump::Delegate * aDelegate = 0x0000003c`cefcf610)+0x70 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\glue\messagepump.cpp @ 302]
0000003c`cefcf350 00007ffe`d15abb52 xul!MessageLoop::RunHandler(void)+0x1b [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\chromium\src\base\message_loop.cc @ 226]
           ^ User interrupted operation error in '~* kp'
2:031> !analyze -v -f
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\system32\ole32.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Windows\System32\msmpeg2vdec.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\system32\mf.dll - 
*** ERROR: Module load completed but symbols could not be loaded for C:\Program Files (x86)\Malwarebytes Anti-Exploit\mbae64.dll
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\Stardock\Start8\Start8_64.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\SYSTEM32\nvwgf2umx.dll - 

FAULTING_IP: 
xul!mozilla::MediaManager::OnNavigation+1de [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\media\mediamanager.cpp @ 2698]
00007ffe`d2707856 4c3998c8000000  cmp     qword ptr [rax+0C8h],r11

EXCEPTION_RECORD:  ffffffffffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 00007ffed2707856 (xul!mozilla::MediaManager::OnNavigation+0x00000000000001de)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 0000000000000000
   Parameter[1]: ffffffffffffffff
Attempt to read from address ffffffffffffffff

FAULTING_THREAD:  0000000000003fc4

PROCESS_NAME:  firefox.exe

ERROR_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%08lx referenced memory at 0x%08lx. The memory could not be %s.

EXCEPTION_PARAMETER1:  0000000000000000

EXCEPTION_PARAMETER2:  ffffffffffffffff

READ_ADDRESS:  ffffffffffffffff 

FOLLOWUP_IP: 
xul!mozilla::MediaManager::OnNavigation+1de [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\media\mediamanager.cpp @ 2698]
00007ffe`d2707856 4c3998c8000000  cmp     qword ptr [rax+0C8h],r11

NTGLOBALFLAG:  70

APPLICATION_VERIFIER_FLAGS:  0

BUGCHECK_STR:  APPLICATION_FAULT_INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

PRIMARY_PROBLEM_CLASS:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

DEFAULT_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

LAST_CONTROL_TRANSFER:  from 00007ffed216dabc to 00007ffed2707856

STACK_TEXT:  
0000003c`cefce510 00007ffe`d216dabc : 00000000`00000000 00000000`8000006e 0000003c`d9043800 0000003c`cefce690 : xul!mozilla::MediaManager::OnNavigation+0x1de [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\media\mediamanager.cpp @ 2698]
0000003c`cefce560 00007ffe`d194366a : 0000003c`d9043800 00000000`00000000 0000003c`d9043800 0000003c`d9098000 : xul!mozilla::dom::Navigator::OnNavigation+0x28 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\base\navigator.cpp @ 1778]
0000003c`cefce590 00007ffe`d14d8453 : 0000003c`d9043830 0000003c`d9043830 0000003c`cefce908 00000000`00000000 : xul!nsGlobalWindow::SetNewDocument+0x8bf5be [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\base\nsglobalwindow.cpp @ 2700]
0000003c`cefce8b0 00007ffe`d14d6b9e : 00000000`00000001 0000003c`d92ed000 00000000`00000000 0000003c`dde40c20 : xul!nsDocumentViewer::InitInternal+0x25b [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\layout\base\nsdocumentviewer.cpp @ 900]
0000003c`cefce950 00007ffe`d131cac5 : 0000003c`d6042ec0 0000003c`d6042ec0 0000003c`cefceaa0 0000003c`d9043800 : xul!nsDocumentViewer::Init+0x1e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\layout\base\nsdocumentviewer.cpp @ 644]
0000003c`cefce9a0 00007ffe`d10789df : 0000003c`00000000 0000003c`d8f04a60 0000003c`d90439c8 0000003c`d90439c8 : xul!nsDocShell::SetupNewViewer+0x355 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 9382]
0000003c`cefceb60 00007ffe`d14b69f4 : 0000003c`d0f498e0 00000000`80000000 0000003c`d9043800 00007ffe`d1721562 : xul!nsDocShell::Embed+0x2f [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 7241]
0000003c`cefceb90 00007ffe`d14b6658 : 00000000`00000000 00000000`00000000 0000003c`dde05dd0 0000003c`dde05dd0 : xul!nsDocShell::CreateContentViewer+0x158 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdocshell.cpp @ 9187]
0000003c`cefcec40 00007ffe`d14b641a : 00000000`00790800 0000003c`d89478b8 0000003c`d89478a0 00000000`00000000 : xul!nsDSURIContentListener::DoContent+0xe8 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\docshell\base\nsdsuricontentlistener.cpp @ 128]
0000003c`cefcecb0 00007ffe`d14b54d5 : 0000003c`d62f7301 00000000`00000001 0000003c`e7000620 0000003c`cf300200 : xul!nsDocumentOpenInfo::TryContentListener+0x2fe [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 742]
0000003c`cefced80 00007ffe`d11a847b : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : xul!nsDocumentOpenInfo::DispatchContent+0x34d [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 414]
0000003c`cefcefa0 00007ffe`d11a7b7f : 00000000`00000000 0000003c`d0f49918 0000003c`cefcf128 00000000`00003682 : xul!nsDocumentOpenInfo::OnStartRequest+0x353 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\uriloader\base\nsuriloader.cpp @ 277]
0000003c`cefcf0c0 00007ffe`d11cbf83 : 0000003c`db542d70 0000003c`db542d70 0000003c`db542d70 0000003c`d8a4bca8 : xul!nsBaseChannel::OnStartRequest+0x7f [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsbasechannel.cpp @ 809]
0000003c`cefcf0f0 00007ffe`d11ccc71 : 0000003c`d0f3f000 00000000`00003682 0000003c`db542d78 0000003c`d8a4bca8 : xul!nsInputStreamPump::OnStateStart+0xc3 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsinputstreampump.cpp @ 525]
0000003c`cefcf120 00007ffe`d11ccb64 : 0000003c`d0f3f000 0000003c`d6047d68 0000003c`cefcf239 0000003c`d0f093d0 : xul!nsInputStreamPump::OnInputStreamReady+0xf9 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\netwerk\base\nsinputstreampump.cpp @ 441]
0000003c`cefcf160 00007ffe`d120a83f : 0000003c`d0f8a800 0000003c`cefcf239 0000003c`cefcf301 00000000`00000001 : xul!nsOutputStreamReadyEvent::Run+0x24 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\xpcom\io\nsstreamutils.cpp @ 97]
0000003c`cefcf190 00007ffe`d12087c5 : 0000003c`d0f12ce0 0000003c`d0f12ce0 0000003c`cefcf5f0 0000003c`d0f12ce0 : xul!nsThread::ProcessNextEvent+0x4b3 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\xpcom\threads\nsthread.cpp @ 1216]
0000003c`cefcf2a0 00007ffe`d1d3f600 : 0000003c`d0f12ce0 0000003c`cefcf601 0000003c`cefcf501 0000003c`cefcf7a0 : xul!mozilla::ipc::MessagePump::Run+0x9d [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\glue\messagepump.cpp @ 96]
0000003c`cefcf320 00007ffe`d15abb8f : 0000003c`cefcf5f0 0000003c`cefcf610 0000003c`d0f02101 00007ffe`dea1854d : xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x70 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\glue\messagepump.cpp @ 302]
0000003c`cefcf350 00007ffe`d15abb52 : 0000003c`cefcf3d8 0000003c`d0f02098 00007ffe`d3f04af0 00007ffe`d1206e3e : xul!MessageLoop::RunHandler+0x1b [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\chromium\src\base\message_loop.cc @ 226]
0000003c`cefcf380 00007ffe`d14945a4 : 0000003c`d8a4bca0 0000003c`d901dd60 0000003c`cefcf610 0000003c`d0f02101 : xul!MessageLoop::Run+0x3e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\chromium\src\base\message_loop.cc @ 206]
0000003c`cefcf3d0 00007ffe`d1494264 : 0000003c`d8a4bca0 00007ffe`d1454f8a 0000edeb`f46a4bc1 0000003c`d8a4bca0 : xul!nsBaseAppShell::Run+0x3c [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\widget\nsbaseappshell.cpp @ 158]
0000003c`cefcf400 00007ffe`d2ead0a6 : 00000000`00000003 0000003c`d0f02101 0000003c`cefcf4a9 00007ffe`d2eac2fd : xul!nsAppShell::Run+0x2c [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\widget\windows\nsappshell.cpp @ 264]
0000003c`cefcf430 00007ffe`d1d3f5b9 : 0000003c`d8a4bca0 00000000`00000000 00000000`00000002 00007ffe`d35896f8 : xul!XRE_RunAppShell+0x2e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\toolkit\xre\nsembedfunctions.cpp @ 869]
0000003c`cefcf460 00007ffe`d15abb8f : 0000003c`cefcf5f0 0000003c`cefcf610 0000003c`d0f02101 0000003c`cefcf4b0 : xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x29 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\glue\messagepump.cpp @ 278]
0000003c`cefcf490 00007ffe`d15abb52 : 0000003c`00000fff 0000003c`cefcf7a0 00000000`00000003 00007ffe`d15db476 : xul!MessageLoop::RunHandler+0x1b [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\chromium\src\base\message_loop.cc @ 226]
0000003c`cefcf4c0 00007ffe`d2eacd2d : 0000003c`d8a40400 0000003c`cefcf610 00000000`00000003 0000003c`cefcf7a0 : xul!MessageLoop::Run+0x3e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\chromium\src\base\message_loop.cc @ 206]
0000003c`cefcf510 00007ff6`3449b116 : 00000000`0000000b 0000003c`d0f020e0 0000003c`d0f02080 00007ff6`344b68a0 : xul!XRE_InitChildProcess+0x639 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\toolkit\xre\nsembedfunctions.cpp @ 705]
0000003c`cefcf780 00007ff6`34498669 : 00000000`0000000c 00000000`0000000c 0000003c`d0f020e0 00000000`00000480 : firefox!content_process_main+0x8e [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\ipc\contentproc\plugin-container.cpp @ 198]
0000003c`cefcf7d0 00007ff6`34496615 : 00000000`00000000 00007ff6`3380f000 00000000`00000000 00000000`00000000 : firefox!wmain+0x54f9 [c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\toolkit\xre\nswindowswmain.cpp @ 115]
0000003c`cefcfbd0 00007ffe`f47113d2 : 00007ff6`34496670 00007ff6`3380f000 00000000`00000000 00000000`00000000 : firefox!__scrt_common_main_seh+0x11d [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 253]
0000003c`cefcfc10 00007ffe`f68654e4 : 00007ffe`f47113b0 00000000`00000000 00000000`00000000 00000000`00000000 : KERNEL32!BaseThreadInitThunk+0x22
0000003c`cefcfc40 00000000`00000000 : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ntdll!RtlUserThreadStart+0x34


FAULTING_SOURCE_CODE:  
No source found for 'c:\builds\moz2_slave\m-cen-w64-ntly-000000000000000\build\src\dom\media\mediamanager.cpp'


SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  xul!mozilla::MediaManager::OnNavigation+1de

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: xul

IMAGE_NAME:  xul.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  5829c841

STACK_COMMAND:  ~31s ; kb

FAILURE_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5_c0000005_xul.dll!mozilla::MediaManager::OnNavigation

BUCKET_ID:  X64_APPLICATION_FAULT_INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5_xul!mozilla::MediaManager::OnNavigation+1de

WATSON_STAGEONE_URL:  http://watson.microsoft.com/StageOne/firefox_exe/53_0_0_6162/5829c279/xul_dll/53_0_0_6162/5829c841/c0000005/01717856.htm?Retriage=1

Followup: MachineOwner
Flags: sec-bounty?
Group: core-security → media-core-security
2698 seems to be  RemoveMediaDevicesCallback(aWindowID);
Rank: 10
Keywords: csectype-uaf, sec-high
Priority: -- → P1
Attached patch imported patch — — Splinter Review 
The Clear()s are unneeded/useless.  We could make it an nsTArray<RefPtr<>>, but that would require changing the callback type to a MediaDevices object - which is worth considering as an alternative
Attachment #8812538 - Flags: review?(jib)
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Comment on attachment 8812538 [details] [diff] [review]
imported patch

Review of attachment 8812538 [details] [diff] [review]:
-----------------------------------------------------------------

Lifting mCallbackMutex out to walk the list safely is a good idea, but AFAIK this particular list isn't modified off mainthread (only read), which makes me doubt it could have caused this main-thread crash.

Instead, I suspect you're on to something with the ref-counting. MediaManager doesn't hold any references to the navigator.mediaDevices objects, which are cycle-collected reference-counted objects, instead it keeps raw pointers, which I think is the problem.

We can change that as you suggest, or call RemoveDeviceChangeCallback from ~MediaDevices() (which as I recall, we did in an early patch version of this feature, but I expressed concern that removal in GC wasn't soon enough, so we moved it to OnNavigation, which is probably always sooner in practice, except with fuzzing. In hindsight we should of course have called it from both places).

Munro, do you agree? Want to craft a patch?
Attachment #8812538 - Flags: review?(jib) → review+
Flags: needinfo?(mchiang)
(In reply to Randell Jesup [:jesup] from comment #3)
> The Clear()s are unneeded/useless.  We could make it an nsTArray<RefPtr<>>,
> but that would require changing the callback type to a MediaDevices object -
> which is worth considering as an alternative

(In reply to Jan-Ivar Bruaroey [:jib] from comment #4)
> We can change that as you suggest, or call RemoveDeviceChangeCallback from
> ~MediaDevices() (which as I recall, we did in an early patch version of this
> feature, but I expressed concern that removal in GC wasn't soon enough, so
> we moved it to OnNavigation, which is probably always sooner in practice,
> except with fuzzing. In hindsight we should of course have called it from
> both places).
> 
> Munro, do you agree? Want to craft a patch?

It's not easy to make it an nsTArray<RefPtr<>> because there are three classes (MediaDevices / MediaManager / CamerasChild) are derived from DeviceChangeCallback.

I can add a snippet in ~MediaDevices() to call RemoveDeviceChangeCallback as jib suggested. If there is no concern, I will file a new extension bug and submit the patch.
Flags: needinfo?(mchiang)
Agree. Thanks, and you can just add your patch to this bug I think (we should avoid a public bug I think since it's a security issue).
Comment on attachment 8812674 [details] [diff] [review]
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch

Review of attachment 8812674 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm provided we remove the assert.

::: dom/media/MediaDevices.cpp
@@ +139,5 @@
>  
> +MediaDevices::~MediaDevices()
> +{
> +  MediaManager* mediamanager = MediaManager::GetIfExists();
> +  MOZ_ASSERT(mediamanager != nullptr);

We can't assert here I don't think, as navigator.mediaDevices can exist without MediaManager which is only created as needed.
Attachment #8812674 - Flags: review?(jib) → review+
This needs sec approval before it can land (or clarification that only Trunk is affected).
https://wiki.mozilla.org/Security/Bug_Approval_Process
Keywords: checkin-needed
Assignee: rjesup → mchiang
I think we still want my patch - the Clears are redundant, and without thread assertions there's risk of future changes making this fail, so locking around the entire list access makes sense.  Also, this will cause potential issues with static analysis.

I presume the off-thread access to this list doesn't change the data pointed to by the list entries in any way visible to the code that reads it without a lock on MainThread.

As ryan says, all sec-high and sec-crits that are in any branch other than Nightly need sec-approval before landing.
Flags: needinfo?(jib)
(In reply to Randell Jesup [:jesup] from comment #14)

> I presume the off-thread access to this list doesn't change the data pointed
> to by the list entries in any way visible to the code that reads it without
> a lock on MainThread.

So MediaEngineWebRTC::Shutdown() calls RemoveDeviceChangeCallback() for CamerasChild, and does so off-main-thread (via ShutdownTask::Run(), on the media thread).  So I think the assertion there doesn't hold.  We should take both patches.
Comment on attachment 8812704 [details] [diff] [review]
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch

Applies to both patches.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? 

Even though the timing window is probably quite small here, and can't easily be widened AFAIK (delete navigator.mediaDevices didn't work from content script) this is still a Use-After-Free, so with a timely GC, I imagine it is still subject to the regular UAF exploits.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No more so than the code.

Which older supported branches are affected by this flaw? 51 and 52

If not all supported branches, which bug introduced the flaw? Bug 1286429

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Should be the same.

How likely is this patch to cause regressions; how much testing does it need?

This is all proper intuitive cleanup and should need only basic testing.
Flags: needinfo?(jib)
Attachment #8812704 - Flags: sec-approval?
Comment on attachment 8812538 [details] [diff] [review]
imported patch

Same
Attachment #8812538 - Flags: sec-approval?
(In reply to Randell Jesup [:jesup] from comment #15)
> (In reply to Randell Jesup [:jesup] from comment #14)
> So MediaEngineWebRTC::Shutdown() calls RemoveDeviceChangeCallback() for
> CamerasChild, and does so off-main-thread (via ShutdownTask::Run(), on the
> media thread).  So I think the assertion there doesn't hold.  We should take
> both patches.

Note that CamerasChild and MediaManager classes each derive from DeviceChangeCallback independently, so changes in MediaManager::RemoveMediaDevicesCallback do not affect CamerasChild.

Regardless, it's a safe and correct patch to take I think.
Though they do form a callback tree, which means OnDeviceChange() [1] may be called off thread () during MediaManager::RemoveMediaDevicesCallback if I understand correctly, so yes both patches please.

[1] https://dxr.mozilla.org/mozilla-central/rev/0534254e9a40b4bade2577c631fe4cfa0b5db41d/dom/media/MediaManager.cpp#1957
Sorry I'm confusing myself with the various instances. There's a dispatch to main thread in that last link, which means the list in MediaManager is not walked in another thread. Either way.
Tracking 53+ for this sec high issue.
tracking-firefox53: ? → +
Comment on attachment 8812704 [details] [diff] [review]
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch

sec-approval+ for trunk. Please nominate Beta and Aurora patches so we can avoid shipping this flaw.
Attachment #8812704 - Flags: sec-approval? → sec-approval+
Attachment #8812538 - Flags: sec-approval? → sec-approval+
Comment on attachment 8812704 [details] [diff] [review]
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1286429

[User impact if declined]: Possibly exploitable UAF

[Describe test coverage new/current, TreeHerder]: Tight timing hole found with a fuzzer; no direct coverage

[Risks and why]: Low risk changes - add a wider lock around a lost access, ensure when a MediaDevices object is destroyed that it has been unregistered.

[String/UUID change made/needed]: none
Attachment #8812704 - Flags: approval-mozilla-beta?
Attachment #8812704 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/01b543e78ae8
https://hg.mozilla.org/mozilla-central/rev/70f538269f50
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8812704 [details] [diff] [review]
Bug-1317670-call-MediaManager-RemoveDeviceChangeCallback-in-MediaDevices-dtor.patch

Fix a sec-high. Beta51+ and Aurora52+. Should be in 51 beta 4.
Attachment #8812704 - Flags: approval-mozilla-beta?
Attachment #8812704 - Flags: approval-mozilla-beta+
Attachment #8812704 - Flags: approval-mozilla-aurora?
Attachment #8812704 - Flags: approval-mozilla-aurora+
Group: media-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: