1422215 - WebRTC - Use After Free in in JsepSessionImpl::CheckNegotiationNeeded()

Reporter

Description

•

7 years ago

Attached file UAF_CheckNegotiationNeeded_PoC.html — Details

Reproduction test case: UAF_CheckNegotiationNeeded_PoC.html

Steps to reproduce: 
	1. Open UAF_CheckNegotiationNeeded_PoC.html in Firefox browser.
    2. Firefox crashes in JsepSessionImpl::CheckNegotiationNeeded() by accessing freed memory.

		(49a0.5fbc): Access violation - code c0000005 (!!! second chance !!!)
		eax=e5e5e5e5 ebx=1589df0c ecx=e5e5e5e5 edx=00000001 esi=e5e5e5e5 edi=1a9d0740
		eip=62fdbd20 esp=008fcf28 ebp=008fd2f8 iopl=0         nv up ei pl zr na pe nc
		cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
		xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+0x199:
		62fdbd20 8b06            mov     eax,dword ptr [esi]  ds:002b:e5e5e5e5=????????


Firefox version: 59.0a1 (2017-11-30) (32-bit)
OS: Windows 10 

Stack trace:

	(49a0.5fbc): Access violation - code c0000005 (!!! second chance !!!)
	eax=e5e5e5e5 ebx=1589df0c ecx=e5e5e5e5 edx=00000001 esi=e5e5e5e5 edi=1a9d0740
	eip=62fdbd20 esp=008fcf28 ebp=008fd2f8 iopl=0         nv up ei pl zr na pe nc
	cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
	xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+0x199:
	62fdbd20 8b06            mov     eax,dword ptr [esi]  ds:002b:e5e5e5e5=????????
	5:216> !analyze -v
	*******************************************************************************
	*                                                                             *
	*                        Exception Analysis                                   *
	*                                                                             *
	*******************************************************************************

	FAULTING_IP: 
	xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+199 [z:\build\build\src\media\webrtc\signaling\src\jsep\jsepsessionimpl.cpp @ 2335]
	62fdbd20 8b06            mov     eax,dword ptr [esi]

	EXCEPTION_RECORD:  (.exr -1)
	ExceptionAddress: 62fdbd20 (xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+0x00000199)
	   ExceptionCode: c0000005 (Access violation)
	  ExceptionFlags: 00000000
	NumberParameters: 2
	   Parameter[0]: 00000000
	   Parameter[1]: e5e5e5e5
	Attempt to read from address e5e5e5e5

	FAULTING_THREAD:  00005fbc

	PROCESS_NAME:  firefox.exe

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

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

	EXCEPTION_PARAMETER1:  00000000

	EXCEPTION_PARAMETER2:  e5e5e5e5

	READ_ADDRESS:  e5e5e5e5 

	FOLLOWUP_IP: 
	xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+199 [z:\build\build\src\media\webrtc\signaling\src\jsep\jsepsessionimpl.cpp @ 2335]
	62fdbd20 8b06            mov     eax,dword ptr [esi]

	NTGLOBALFLAG:  400

	APPLICATION_VERIFIER_FLAGS:  0

	APP:  firefox.exe

	ANALYSIS_VERSION: 10.0.10240.9 x86fre

	BUGCHECK_STR:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

	DEFAULT_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

	LAST_CONTROL_TRANSFER:  from 63344c5f to 62fdbd20

	STACK_TEXT:  
	008fd2f8 63344c5f 00000000 008fd38c 1b814520 xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+0x199
	008fd310 62521b65 08012800 008fd358 1f6a4c80 xul!mozilla::dom::PeerConnectionImplBinding::checkNegotiationNeeded+0x1e
	008fd368 2db25eb1 08012800 00000000 008fd390 xul!mozilla::dom::GenericBindingMethod+0xec
	WARNING: Frame IP not in any known module. Following frames may be wrong.
	008fd3a4 2da52bd8 00004021 16ff7b20 ffffff8c 0x2db25eb1
	008fd3ec 2da422d9 00001842 12a4c9d0 00000000 0x2da52bd8
	008fd414 2da77300 00004021 09ed6c40 ffffff8c 0x2da422d9
	008fd45c 2da006dd 00001c43 12a543d0 00000000 0x2da77300
	008fd490 62524699 2da77190 00000001 008fd990 0x2da006dd
	008fd748 6252454a 2da77190 08012800 00000000 xul!EnterJit+0x112
	008fd764 629d6279 008fd89c 008fd89c 008fd89c xul!js::jit::MaybeEnterJit+0x54
	008fd810 6231a0a3 08012800 008fd890 09ed7dc0 xul!js::RunScript+0x219
	008fd8ac 6224eb32 00000000 6e1730c0 08012800 xul!js::InternalCallOrConstruct+0x1e3
	008fd8cc 6224f9ad 008fda10 008fda20 008fda30 xul!js::Call+0x8b
	008fd9dc 6333c2b9 008fda20 008fda10 008fda30 xul!JS::Call+0xf7
	008fdb00 630117a2 008fdb38 00000000 008fdcb0 xul!mozilla::dom::PeerConnectionObserverJSImpl::OnSetRemoteDescriptionSuccess+0x11c
	008fdc0c 6333e2be 1512e80c 08012800 00000000 xul!mozilla::PeerConnectionImpl::SetRemoteDescription+0x4a3
	008fdc84 6335124c 00000001 008fdcc0 008fdcb0 xul!mozilla::PeerConnectionImpl::SetRemoteDescription+0x3a
	008fdd58 62521b65 08012800 008fdda0 1f6a4c80 xul!mozilla::dom::PeerConnectionImplBinding::setRemoteDescription+0xb7
	008fddb0 2db25eb1 08012800 00000002 008fddd8 xul!mozilla::dom::GenericBindingMethod+0xec
	008fddfc 2da75297 00004821 1e670440 ffffff86 0x2db25eb1
	008fde4c 2da006dd 00002c43 1650d020 00000002 0x2da75297
	008fde90 62524699 2da75080 00000003 008fe3b8 0x2da006dd
	008fe148 6252454a 2da75080 08012800 00000000 xul!EnterJit+0x112
	008fe164 629d6279 008fe29c 008fe29c 008fe29c xul!js::jit::MaybeEnterJit+0x54
	008fe210 6231a0a3 08012800 008fe290 008fe3c0 xul!js::RunScript+0x219
	008fe2ac 6224eb32 00000000 08012800 080073e8 xul!js::InternalCallOrConstruct+0x1e3
	008fe2cc 623573cb 008fe388 64f2cf98 008fe388 xul!js::Call+0x8b
	008fe3d8 62356dff 008fe42c 008fe408 1638c1b8 xul!js::PromiseObject::create+0x575
	008fe440 2da4ed41 08012800 00000001 008fe468 xul!PromiseConstructor+0xf4
	008fe48c 2da7338f 00004821 12a15480 ffffff8c 0x2da4ed41
	008fe4dc 2db25696 00005041 1650b6a0 00000000 0x2da7338f
	008fe53c 2da006dd 00001c43 0a074520 00000001 0x2db25696
	008fe570 62524699 2db25250 00000002 008fea28 0x2da006dd
	008fe828 6252454a 2db25250 08012800 00000000 xul!EnterJit+0x112
	008fe844 629d6279 008fe97c 008fe97c 008fe97c xul!js::jit::MaybeEnterJit+0x54
	008fe8f0 6231a0a3 08012800 008fe970 008fea00 xul!js::RunScript+0x219
	008fe98c 6224fcd3 00000000 08012800 008feaa0 xul!js::InternalCallOrConstruct+0x1e3
	008fe9b0 62473294 08012800 08012800 008fea14 xul!InternalCall+0x73
	008fea5c 624734d6 008feab4 008feaa0 008fea90 xul!AsyncFunctionResume+0x1ae
	008fead0 62564ec5 008feaf4 008fed48 08012800 xul!AsyncFunctionPromiseReactionJob+0xb4
	008febcc 62319ffd 08012800 00000000 008fed48 xul!PromiseReactionJob+0x37f
	008fec6c 6224eb32 00000000 6e1730c0 08012800 xul!js::InternalCallOrConstruct+0x13d
	008fec8c 6224f9ad 008fede0 64f2cf98 008fedf0 xul!js::Call+0x8b
	008fed9c 62250744 64f2cf98 008fede0 008fedf0 xul!JS::Call+0xf7
	008feed4 622f7bbe 15871c20 16d642e0 1a9ce850 xul!mozilla::PromiseJobRunnable::Run+0x125
	008fef30 622280cf 008ff190 158f14c0 07b0e68c xul!mozilla::dom::Promise::PerformMicroTaskCheckpoint+0x83
	008ff0b8 6238a069 158f14c0 02db04a0 008ff200 xul!nsGlobalWindowInner::RunTimeoutHandler+0x1fc
	008ff1c8 624ae9f5 008ff218 158f14c0 02d970b0 xul!mozilla::dom::TimeoutManager::RunTimeout+0x2e0
	008ff234 624aef2d 008ff268 62349bdc 158086c0 xul!mozilla::dom::TimeoutExecutor::MaybeExecute+0x9f
	008ff23c 62349bdc 158086c0 008ff818 16d78440 xul!mozilla::dom::TimeoutExecutor::Run+0x11
	008ff268 62349b7e 64712764 008ff2b4 62c90079 xul!mozilla::ThrottledEventQueue::Inner::ExecuteRunnable+0x57
	008ff274 62c90079 16d64840 008ff818 02d37ca0 xul!mozilla::ThrottledEventQueue::Inner::Executor::Run+0xf
	008ff2b4 622f6b09 16d78440 02d01230 02d01220 xul!mozilla::SchedulerGroup::Runnable::Run+0x4f
	008ff824 6232b3f7 02d37ca0 00000000 008ff84f xul!nsThread::ProcessNextEvent+0x294
	008ff850 62e37bfb 008ff9c8 008ff9c8 02d03040 xul!mozilla::ipc::MessagePump::Run+0x75
	008ff86c 627760c9 008ff9c8 8ce35d3d 07b0ed30 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x58
	008ff8a4 62776089 02d37ca0 00000002 02d03000 xul!MessageLoop::RunHandler+0x1f
	008ff8c4 625ee58a 02d01220 008ff9c8 008ff8e4 xul!MessageLoop::Run+0x19
	008ff8d4 625ee316 07b0ed30 02d01220 008ff8f8 xul!nsBaseAppShell::Run+0x34
	008ff8e4 644a9b88 07b0ed30 02d01220 07b0ed30 xul!nsAppShell::Run+0x26
	008ff8f8 62e37bb9 008ff9c8 02d01220 008ff940 xul!XRE_RunAppShell+0x2f
	008ff908 627760c9 008ff9c8 8ce35cd9 00000013 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x16
	008ff940 62776089 02d19800 00000001 008ff900 xul!MessageLoop::RunHandler+0x1f
	008ff960 644a9a12 02d060f0 00000016 02d03040 xul!MessageLoop::Run+0x19
	008ffa84 644ac767 008ffab0 008ffab8 002e93d3 xul!XRE_InitChildProcess+0x4bd
	008ffa90 002e93d3 00000016 02d03040 008ffab0 xul!mozilla::BootstrapImpl::XRE_InitChildProcess+0x11
	008ffab8 002e6558 02d03040 029f2b00 754580e8 firefox!content_process_main+0x74
	008ffe10 002e5229 00000017 ffcefac0 029f3610 firefox!wmain+0x54f8
	008ffe58 75808654 00603000 75808630 9caec81a firefox!__scrt_common_main_seh+0xf8
	008ffe6c 77584a47 00603000 9e45a4f5 00000000 KERNEL32!BaseThreadInitThunk+0x24
	008ffeb4 77584a17 ffffffff 775a9eb7 00000000 ntdll!__RtlUserThreadStart+0x2f
	008ffec4 00000000 002e529f 00603000 00000000 ntdll!_RtlUserThreadStart+0x1b


	FAULTING_SOURCE_LINE:  z:\build\build\src\media\webrtc\signaling\src\jsep\jsepsessionimpl.cpp

	FAULTING_SOURCE_FILE:  z:\build\build\src\media\webrtc\signaling\src\jsep\jsepsessionimpl.cpp

	FAULTING_SOURCE_LINE_NUMBER:  2335

	SYMBOL_STACK_INDEX:  0

	SYMBOL_NAME:  xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+199

	FOLLOWUP_NAME:  MachineOwner

	MODULE_NAME: xul

	IMAGE_NAME:  xul.dll

	DEBUG_FLR_IMAGE_TIMESTAMP:  5a209432

	STACK_COMMAND:  ~216s ; kb

	BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5_xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+199

	PRIMARY_PROBLEM_CLASS:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5_xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+199

	FAILURE_PROBLEM_CLASS:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5

	FAILURE_EXCEPTION_CODE:  c0000005

	FAILURE_IMAGE_NAME:  xul.dll

	FAILURE_FUNCTION_NAME:  mozilla::JsepSessionImpl::CheckNegotiationNeeded

	FAILURE_SYMBOL_NAME:  xul.dll!mozilla::JsepSessionImpl::CheckNegotiationNeeded

	FAILURE_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_e5e5e5e5_c0000005_xul.dll!mozilla::JsepSessionImpl::CheckNegotiationNeeded

	ANALYSIS_SOURCE:  UM

	FAILURE_ID_HASH_STRING:  um:invalid_pointer_read_fill_pattern_e5e5e5e5_c0000005_xul.dll!mozilla::jsepsessionimpl::checknegotiationneeded

	FAILURE_ID_HASH:  {db194bc4-bad6-695b-bb0a-3c888ed6e184}

	Followup:     MachineOwner
	---------

Ryan VanderMeulen [:RyanVM]

Comment 1

•

7 years ago

I've been unable to reproduce the crash on Win10, but can on Ubuntu 17.10 if I let the PoC run for ~30-60s. I've had a bit of trouble getting a useful regression range, but I *think* it was around late October from around the time bug 1405940 landed. Can you please take a look, Michael? (And feel free to change status flags where applicable if I got that regression range wrong)

Group: core-security → media-core-security

Has Regression Range: --- → yes

status-firefox57: --- → unaffected

status-firefox58: --- → affected

status-firefox-esr52: --- → unaffected

tracking-firefox58: --- → ?

tracking-firefox59: --- → ?

Flags: needinfo?(mfroman)

Keywords: crash, csectype-uaf, regression

Version: 59 Branch → unspecified

Michael Froman [:mjf]

Comment 2

•

7 years ago

If I'm reading correctly, the crash above is with 59.01 from 11/30 which is after the transceiver work landed which has changed things quite a bit.  Byron, any thoughts on what is going on here?

Flags: needinfo?(mfroman) → needinfo?(docfaraday)

Michael Froman [:mjf]

Comment 3

•

7 years ago

In the meantime, I have a debug build going on my linux box.  Once that is complete I'll try it locally, and then see if I can catch it in rr.

Byron Campen [:bwc]

Assignee

Comment 4

•

7 years ago

Yeah, this may be a transceivers bug. Looking now.

Flags: needinfo?(docfaraday)

Julien Cristau [:jcristau]

Updated

•

7 years ago

tracking-firefox58: ? → +

tracking-firefox59: ? → +

Nils Ohlmeier [:drno]

Updated

•

7 years ago

Rank: 7

Priority: -- → P1

Nils Ohlmeier [:drno]

Comment 5

•

7 years ago

P1 bugs need owners. Assigning to Byron since you were last saying that you are going to take a look.

Assignee: nobody → docfaraday

Looben Yang

Reporter

Comment 6

•

7 years ago

Attached file UAF_CheckNegotiationNeeded_PoC_EIP_41414141.js — Details

Attached a PoC  UAF_CheckNegotiationNeeded_PoC_EIP_41414141.js with EIP control to demonstrate the clear exploitability.

 	Steps to run:
	
	1. Run server side script UAF_CheckNegotiationNeeded_PoC_EIP_41414141.js in Node.js (node UAF_CheckNegotiationNeeded_PoC_EIP_41414141.js ).
	2. Enter http://localhost:12345 
        3. Firefox craches by executing code at location 0x41414141



Firefox version: 59.0a1 (2017-12-04) (32-bit)
OS: Windows 10 


(7428.3650): Access violation - code c0000005 (!!! second chance !!!)
eax=a1a1a1a1 ebx=236c97b4 ecx=e5e5e5e5 edx=00000001 esi=e5e5e5e5 edi=08404ea0
eip=41414141 esp=010fc014 ebp=010fc3e8 iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
41414141 ??              ???
3:159> !analyze -v
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************


FAULTING_IP: 
unknown!noop+0
41414141 ??              ???

EXCEPTION_RECORD:  (.exr -1)
ExceptionAddress: 41414141
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000008
   Parameter[1]: 41414141
Attempt to execute non-executable address 41414141

FAULTING_THREAD:  00003650

PROCESS_NAME:  firefox.exe

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

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

EXCEPTION_PARAMETER1:  00000008

EXCEPTION_PARAMETER2:  41414141

WRITE_ADDRESS:  41414141 

FOLLOWUP_IP: 
unknown!noop+0
41414141 ??              ???

FAILED_INSTRUCTION_ADDRESS: 
unknown!noop+0
41414141 ??              ???

NTGLOBALFLAG:  400

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

ANALYSIS_VERSION: 10.0.10240.9 x86fre

IP_ON_HEAP:  41414141
The fault address in not in any loaded module, please check your build's rebase
log at <releasedir>\bin\build_logs\timebuild\ntrebase.log for module which may
contain the address if it were loaded.

IP_IN_FREE_BLOCK: 41414141

BUGCHECK_STR:  SOFTWARE_NX_FAULT_INVALID_CODE

DEFAULT_BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID_CODE

LAST_CONTROL_TRANSFER:  from 107990f6 to 41414141

STACK_TEXT:  
WARNING: Frame IP not in any known module. Following frames may be wrong.
010fc010 107990f6 0000018a 00000000 25cf71c0 0x41414141
010fc3e8 10afe875 00000000 126bb890 25cf7100 xul!mozilla::JsepSessionImpl::CheckNegotiationNeeded+0x19e
010fc400 0fd211e7 08912800 010fc448 24dbb900 xul!mozilla::dom::PeerConnectionImplBinding::checkNegotiationNeeded+0x1e
010fc458 156b18d1 08912800 00000000 010fc480 xul!mozilla::dom::GenericBindingMethod+0xec
010fc494 156d50a8 00004021 250dba60 ffffff8c 0x156b18d1
010fc4dc 155f06dd 00001443 1f6aa2b0 00000000 0x156d50a8
010fc508 0fb88a5d 156d4df0 00000001 0ab1f060 0x155f06dd
010fc7c0 0fb8890d 156d4df0 08912800 00000000 xul!EnterJit+0x112
010fc7dc 101996d9 010fc914 010fc914 010fc914 xul!js::jit::MaybeEnterJit+0x54
010fc888 0faa5b71 08912800 010fc908 010fcae8 xul!js::RunScript+0x219
010fc924 0fb8a283 00000000 08912800 0ab1f058 xul!js::InternalCallOrConstruct+0x1e1
010fc948 0fb8f415 08912800 00000000 08912834 xul!InternalCall+0x73
010fd284 101997aa 010fd3bc 010fd3bc 010fd3bc xul!Interpret+0x445
010fd330 0faa5b71 08912800 010fd3b0 0a69c4b0 xul!js::RunScript+0x2ea
010fd3cc 0fa9d517 00000000 6e1730c0 08912800 xul!js::InternalCallOrConstruct+0x1e1
010fd3ec 0fa9ca7d 010fd530 010fd540 010fd550 xul!js::Call+0x8b
010fd4fc 10af7bf9 010fd540 010fd530 010fd550 xul!JS::Call+0xf7
010fd620 107cfdab 010fd658 00000000 010fd7d0 xul!mozilla::dom::PeerConnectionObserverJSImpl::OnSetRemoteDescriptionSuccess+0x11c
010fd72c 10af9014 24de800c 08912800 00000000 xul!mozilla::PeerConnectionImpl::SetRemoteDescription+0x4a3
010fd7a4 10b0ae0c 00000001 010fd7e0 010fd7d0 xul!mozilla::PeerConnectionImpl::SetRemoteDescription+0x3a
010fd878 0fd211e7 08912800 010fd8c0 24dbb900 xul!mozilla::dom::PeerConnectionImplBinding::setRemoteDescription+0xb7
010fd8d0 156b18d1 08912800 00000002 010fd8f8 xul!mozilla::dom::GenericBindingMethod+0xec
010fd91c 156dcb97 00004821 20459bc0 ffffff86 0x156b18d1
010fd96c 155f06dd 00002c43 0a6d1328 00000002 0x156dcb97
010fd9b0 0fb88a5d 156dc980 00000003 010fded8 0x155f06dd
010fdc68 0fb8890d 156dc980 08912800 00000000 xul!EnterJit+0x112
010fdc84 101996d9 010fddbc 010fddbc 010fddbc xul!js::jit::MaybeEnterJit+0x54
010fdd30 0faa5b71 08912800 010fddb0 010fdee0 xul!js::RunScript+0x219
010fddcc 0fa9d517 00000000 08912800 089073e8 xul!js::InternalCallOrConstruct+0x1e1
010fddec 0fa482eb 010fdea8 126c0e28 010fdea8 xul!js::Call+0x8b
010fdef8 0fa47d1f 010fdf4c 010fdf28 1ee431b8 xul!js::PromiseObject::create+0x575
010fdf60 156bd271 08912800 00000001 010fdf88 xul!PromiseConstructor+0xf4
010fdfac 156dac7f 00004821 1f67c340 ffffff8c 0x156bd271
010fdffc 156b6534 00005041 0a6cf998 00000000 0x156dac7f
010fe05c 155f06dd 00002443 0a874520 00000001 0x156b6534
010fe098 0fb88a5d 156b6130 00000002 010fe550 0x155f06dd
010fe350 0fb8890d 156b6130 08912800 00000000 xul!EnterJit+0x112
010fe36c 101996d9 010fe4a4 010fe4a4 010fe4a4 xul!js::jit::MaybeEnterJit+0x54
010fe418 0faa5b71 08912800 010fe498 010fe528 xul!js::RunScript+0x219
010fe4b4 0fb8a283 00000000 08912800 010fe5e0 xul!js::InternalCallOrConstruct+0x1e1
010fe4d8 0fd2bfc7 08912800 08912800 08912830 xul!InternalCall+0x73
010fe584 1041c4ad 010fe5f4 010fe5e0 010fe5d0 xul!AsyncFunctionResume+0x1ae
010fe59c 0fa7d39c 010fe5f4 010fe5e0 010fe5d0 xul!js::AsyncFunctionAwaitedFulfilled+0x14
010fe610 0fa95a3d 010fe634 010fe888 08912800 xul!AsyncFunctionPromiseReactionJob+0xb8
010fe70c 0faa5acd 08912800 00000000 010fe888 xul!PromiseReactionJob+0x37f
010fe7ac 0fa9d517 00000000 6e1730c0 08912800 xul!js::InternalCallOrConstruct+0x13d
010fe7cc 0fa9ca7d 010fe920 126c0e28 010fe930 xul!js::Call+0x8b
010fe8dc 0fa9b105 126c0e28 010fe920 010fe930 xul!JS::Call+0xf7
010fea14 0fa868ed 236c2540 24dda430 1f868580 xul!mozilla::PromiseJobRunnable::Run+0x125
010fea68 0f9fdd43 010fecd8 1f868580 0fab20f8 xul!mozilla::dom::Promise::PerformMicroTaskCheckpoint+0x5b
010fec00 0f9fd8e6 1f868580 1f8405c0 010fed48 xul!nsGlobalWindowInner::RunTimeoutHandler+0x1f4
010fed10 0f9ff3ac 000fed60 1f868580 08aab2e0 xul!mozilla::dom::TimeoutManager::RunTimeout+0x2e0
010fed78 100ad268 010fedac 0fc419e5 0b752c00 xul!mozilla::dom::TimeoutExecutor::MaybeExecute+0x9f
010fed80 0fc419e5 0b752c00 010ff358 24d0b580 xul!mozilla::dom::TimeoutExecutor::Run+0x11
010fedac 0fc41988 010fedf4 1044b70d 245c2960 xul!mozilla::ThrottledEventQueue::Inner::ExecuteRunnable+0x57
010fedb4 1044b70d 245c2960 010ff358 03737ca0 xul!mozilla::ThrottledEventQueue::Inner::Executor::Run+0xe
010fedf4 0fa8588c 24d0b580 03701230 03701220 xul!mozilla::SchedulerGroup::Runnable::Run+0x4f
010ff360 0fa92ee2 03737ca0 00000000 010ff38b xul!nsThread::ProcessNextEvent+0x294
010ff38c 105f512f 010ff508 010ff508 03703040 xul!mozilla::ipc::MessagePump::Run+0x75
010ff3a8 0fc40adc 010ff508 b39e1db7 08a4ba60 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x58
010ff3e0 0fc40a9c 03737ca0 00000002 03703000 xul!MessageLoop::RunHandler+0x1f
010ff400 0fd829b4 08a4ba60 010ff508 010ff420 xul!MessageLoop::Run+0x19
010ff410 0fd82740 08a4ba60 08a4ba60 010ff438 xul!nsBaseAppShell::Run+0x34
010ff420 11c3f5a0 08a4ba60 010ff508 03701220 xul!nsAppShell::Run+0x26
010ff438 105f50ed 010ff508 03701220 010ff480 xul!XRE_RunAppShell+0x30
010ff448 0fc40adc 010ff508 b39e1ad7 00000013 xul!mozilla::ipc::MessagePumpForChildProcess::Run+0x16
010ff480 0fc40a9c 03719800 00000001 010ff400 xul!MessageLoop::RunHandler+0x1f
010ff4a0 11c3f429 037060f0 00000016 03703040 xul!MessageLoop::Run+0x19
010ff5c0 11c42173 010ff5ec 010ff5f4 000a93a3 xul!XRE_InitChildProcess+0x4bd
010ff5cc 000a93a3 00000016 03703040 010ff5ec xul!mozilla::BootstrapImpl::XRE_InitChildProcess+0x11
010ff5f4 000a6578 03703040 03212c08 754580e8 firefox!content_process_main+0x74
010ff94c 000a5249 00000017 ffb0fbc8 03213710 firefox!wmain+0x5518
010ff994 75808654 00ef6000 75808630 d4cc29b7 firefox!__scrt_common_main_seh+0xf8
010ff9a8 77584a47 00ef6000 d6268c5a 00000000 KERNEL32!BaseThreadInitThunk+0x24
010ff9f0 77584a17 ffffffff 775a9eb8 00000000 ntdll!__RtlUserThreadStart+0x2f
010ffa00 00000000 000a52bf 00ef6000 00000000 ntdll!_RtlUserThreadStart+0x1b


SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  unknown!noop+0

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: unknown

IMAGE_NAME:  unknown.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  0

STACK_COMMAND:  ~159s ; kb

BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID_CODE_BAD_IP_unknown!noop+0

PRIMARY_PROBLEM_CLASS:  SOFTWARE_NX_FAULT_INVALID_CODE_BAD_IP_unknown!noop+0

FAILURE_PROBLEM_CLASS:  SOFTWARE_NX_FAULT_INVALID_CODE

FAILURE_EXCEPTION_CODE:  c0000005

FAILURE_IMAGE_NAME:  unknown.dll

FAILURE_FUNCTION_NAME:  noop

FAILURE_SYMBOL_NAME:  unknown.dll!noop

FAILURE_BUCKET_ID:  SOFTWARE_NX_FAULT_INVALID_CODE_c0000005_unknown.dll!noop

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:software_nx_fault_invalid_code_c0000005_unknown.dll!noop

FAILURE_ID_HASH:  {44389c44-ef1c-c2df-f2c6-035101771881}

Followup:     MachineOwner
---------

Byron Campen [:bwc]

Assignee

Comment 7

•

7 years ago

I'm having no luck reproducing this on a debug linux ASAN build.

Byron Campen [:bwc]

Assignee

Comment 8

•

7 years ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #1)
> I've been unable to reproduce the crash on Win10, but can on Ubuntu 17.10 if
> I let the PoC run for ~30-60s. I've had a bit of trouble getting a useful
> regression range, but I *think* it was around late October from around the
> time bug 1405940 landed. Can you please take a look, Michael? (And feel free
> to change status flags where applicable if I got that regression range wrong)

So you're saying you saw a UAF on the 58 branch? What was the stack? JsepSessionImpl::CheckNegotiationNeeded did not exist back then.

Ryan VanderMeulen [:RyanVM]

Comment 9

•

7 years ago

It was crashing with regular opt builds, seemed to be caused by the audioipc landing. Maybe a different issue.

Byron Campen [:bwc]

Assignee

Comment 10

•

7 years ago

No luck reproducing with a non-ASAN build either.

Byron Campen [:bwc]

Assignee

Comment 11

•

7 years ago

Attached patch Attempt to diagnose — Details — Splinter Review

MozReview-Commit-ID: 1lToGTJEtQe

Byron Campen [:bwc]

Assignee

Comment 12

•

7 years ago

Please attempt to reproduce with the attached patch applied. If you need a binary, I can create one for you with try, just let me know.

Flags: needinfo?(loobenyang)

Looben Yang

Reporter

Comment 13

•

7 years ago

(In reply to Byron Campen [:bwc] from comment #12)
> Please attempt to reproduce with the attached patch applied. If you need a
> binary, I can create one for you with try, just let me know.

Yes, please create binary if you can.

Flags: needinfo?(loobenyang)

Byron Campen [:bwc]

Assignee

Comment 14

•

7 years ago

Ok, try push is here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6525862a63b37ed5a451fdb2627dd2e8a1b117ae

Looben Yang

Reporter

Comment 15

•

7 years ago

Attached file UAF_CheckNegotiationNeeded_PoC2.html — Details

Can you try reproducing it with this new test case UAF_CheckNegotiationNeeded_PoC2.html? Please let me know if you still need me to run  try build. 

I can easily reproduce it in Linux ASAN build with this second test case.

Firefox version: 59.0a1 (2017-12-04) (64-bit)
OS: Ubuntu 14.04 LTS

=================================================================
==8004==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020001e9678 at pc 0x7f48208de203 bp 0x7fffec73ebc0 sp 0x7fffec73ebb8
READ of size 8 at 0x6020001e9678 thread T0 (file:// Content)
    #0 0x7f48208de202 in mozilla::SipccSdp::GetMediaSection(unsigned long) /builds/worker/workspace/build/src/media/webrtc/signaling/src/sdp/SipccSdp.cpp:51:11
    #1 0x7f48207028fc in mozilla::JsepSessionImpl::CheckNegotiationNeeded() const /builds/worker/workspace/build/src/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:2331:33
    #2 0x7f4822644777 in mozilla::dom::PeerConnectionImplBinding::checkNegotiationNeeded(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/PeerConnectionImplBinding.cpp:416:21
    #3 0x7f4823cd7ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
    #4 0x7f47d3bf4e25  (<unknown module>)

0x6020001e9678 is located 0 bytes to the right of 8-byte region [0x6020001e9670,0x6020001e9678)
allocated by thread T0 (file:// Content) here:
    #0 0x4bee13 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88:3
    #1 0x4efe2d in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:84:17
    #2 0x7f48208dfbd6 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:206:12
    #3 0x7f48208dfbd6 in allocate /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/ext/new_allocator.h:104
    #4 0x7f48208dfbd6 in allocate /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/bits/alloc_traits.h:488
    #5 0x7f48208dfbd6 in _M_allocate /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/bits/stl_vector.h:170
    #6 0x7f48208dfbd6 in _M_emplace_back_aux<mozilla::SipccSdpMediaSection *> /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/bits/vector.tcc:412
    #7 0x7f48208dfbd6 in emplace_back<mozilla::SipccSdpMediaSection *> /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/bits/vector.tcc:101
    #8 0x7f48208dfbd6 in push_back /builds/worker/workspace/build/src/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.9.4/../../../../include/c++/4.9.4/bits/stl_vector.h:932
    #9 0x7f48208dfbd6 in mozilla::SipccSdp::Load(sdp_t*, mozilla::SdpErrorHolder&) /builds/worker/workspace/build/src/media/webrtc/signaling/src/sdp/SipccSdp.cpp:128
    #10 0x7f48208fe42e in mozilla::SipccSdpParser::Parse(std::string const&) /builds/worker/workspace/build/src/media/webrtc/signaling/src/sdp/SipccSdpParser.cpp:76:28
    #11 0x7f48206bb2d1 in mozilla::JsepSessionImpl::ParseSdp(std::string const&, mozilla::UniquePtr<mozilla::Sdp, mozilla::DefaultDelete<mozilla::Sdp> >*) /builds/worker/workspace/build/src/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1193:35
    #12 0x7f48206b4460 in mozilla::JsepSessionImpl::SetLocalDescription(mozilla::JsepSdpType, std::string const&) /builds/worker/workspace/build/src/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:678:17
    #13 0x7f48207df4fb in mozilla::PeerConnectionImpl::SetLocalDescription(int, char const*) /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:1750:32
    #14 0x7f482263d5fc in SetLocalDescription /builds/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h:365:10
    #15 0x7f482263d5fc in mozilla::dom::PeerConnectionImplBinding::setLocalDescription(JSContext*, JS::Handle<JSObject*>, mozilla::PeerConnectionImpl*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/PeerConnectionImplBinding.cpp:191
    #16 0x7f4823cd7ad7 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3042:13
    #17 0x7f482a776041 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #18 0x7f482a776041 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:473
    #19 0x7f482a75c0a8 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:528:12
    #20 0x7f482a75c0a8 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3096
    #21 0x7f482a748810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #22 0x7f482a7764ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #23 0x7f482a776fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #24 0x7f482a8705f4 in js::PromiseObject::create(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, bool) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1666:19
    #25 0x7f482a94927f in PromiseConstructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1594:30
    #26 0x7f482a7776a0 in CallJSNative /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:291:15
    #27 0x7f482a7776a0 in CallJSNativeConstructor /builds/worker/workspace/build/src/js/src/jscntxtinlines.h:324
    #28 0x7f482a7776a0 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:568
    #29 0x7f482a75bf22 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3088:18
    #30 0x7f482a748810 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:423:12
    #31 0x7f482a7764ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #32 0x7f482a776fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #33 0x7f482aff6656 in js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jit/VMFunctions.cpp:953:12
    #34 0x7f47d3bec226  (<unknown module>)
    #35 0x7f47d3beb4e7  (<unknown module>)
    #36 0x7f482ad2ff2e in EnterJit /builds/worker/workspace/build/src/js/src/jit/Jit.cpp:99:9
    #37 0x7f482ad2ff2e in js::jit::MaybeEnterJit(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/jit/Jit.cpp:163
    #38 0x7f482a748697 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:408:34
    #39 0x7f482a7764ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:495:15
    #40 0x7f482a776fd2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:541:10
    #41 0x7f482b8c6c6f in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/SelfHosting.cpp:1728:12
    #42 0x7f482b57f63f in AsyncFunctionResume(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>, ResumeKind, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/AsyncFunction.cpp:191:10

SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/workspace/build/src/media/webrtc/signaling/src/sdp/SipccSdp.cpp:51:11 in mozilla::SipccSdp::GetMediaSection(unsigned long)
Shadow bytes around the buggy address:
  0x0c0480035270: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c0480035280: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c0480035290: fa fa fd fd fa fa fd fd fa fa fd fa fa fa 00 00
  0x0c04800352a0: fa fa fd fa fa fa fd fa fa fa 00 fa fa fa fd fa
  0x0c04800352b0: fa fa fd fa fa fa 00 fa fa fa 00 fa fa fa 00 fa
=>0x0c04800352c0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00[fa]
  0x0c04800352d0: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 00 00
  0x0c04800352e0: fa fa 00 05 fa fa 00 05 fa fa fd fa fa fa fd fd
  0x0c04800352f0: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fa
  0x0c0480035300: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c0480035310: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fa
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==8004==ABORTING

Flags: needinfo?(docfaraday)

Byron Campen [:bwc]

Assignee

Comment 16

•

7 years ago

That stack confirms my hypothesis. I expect the binaries in that try push to crash safely with your test-cases. Let me know if something different happens. In the meantime, I will get to the bottom of why some invariants aren't holding.

Flags: needinfo?(docfaraday) → needinfo?(loobenyang)

Byron Campen [:bwc]

Assignee

Comment 17

•

7 years ago

Ugh. We're doing answer validation too late, so some nonsense is slipping past us. Fix is coming.

Byron Campen [:bwc]

Assignee

Comment 18

•

7 years ago

Attached patch Part 2: Do offer/answer validation sooner — Details — Splinter Review

Byron Campen [:bwc]

Assignee

Comment 19

•

7 years ago

Attached patch Part 1: Make logging more consistent in JsepSessionImpl — Details — Splinter Review

Byron Campen [:bwc]

Assignee

Updated

•

7 years ago

Attachment #8934709 - Flags: review?(drno)

Byron Campen [:bwc]

Assignee

Updated

•

7 years ago

Attachment #8934710 - Flags: review?(drno)

Byron Campen [:bwc]

Assignee

Updated

•

7 years ago

Keywords: sec-critical

Byron Campen [:bwc]

Assignee

Comment 20

•

7 years ago

Comment on attachment 8934709 [details] [diff] [review]
Part 2: Do offer/answer validation sooner

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

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

   The patch itself doesn't paint much of a bullseye; it hints that you might be able to cause the internal state to change in dangerous ways by violating offer/answer rules. It might be a little bit of work to target the specific flaw, but would strongly hint that fuzzing could turn something up.

Which older supported branches are affected by this flaw?

   The "late checking" for offer/answer violations is present on all supported branches, but might not be as harmful.

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

   The vulnerability was certainly at least widened by bug 1290948. 

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

   It wouldn't be hard to check these rules earlier in earlier branches, but it is unclear whether this flaw is actually exploitable.

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

   Pretty unlikely, but we will of course run the usual testing.

Attachment #8934709 - Flags: sec-approval?

Nils Ohlmeier [:drno]

Comment 21

•

7 years ago

Comment on attachment 8934710 [details] [diff] [review]
Part 1: Make logging more consistent in JsepSessionImpl

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

LGTM

Only question is if we need to land this as part of this sec bug, or should we land that in a separate open bug?

Attachment #8934710 - Flags: review?(drno) → review+

Nils Ohlmeier [:drno]

Comment 22

•

7 years ago

Comment on attachment 8934709 [details] [diff] [review]
Part 2: Do offer/answer validation sooner

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

LGTM

My only small concern is that you are running ValidateOffer() and ValidateAnswer() on our own offers and answer, which I assume is the point of this change. Have you done a try run to make sure that we are not rejecting our own offers/answers?

Attachment #8934709 - Flags: review?(drno) → review+

Byron Campen [:bwc]

Assignee

Comment 23

•

7 years ago

(In reply to Nils Ohlmeier [:drno] from comment #21)
> Comment on attachment 8934710 [details] [diff] [review]
> Part 1: Make logging more consistent in JsepSessionImpl
> 
> Review of attachment 8934710 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM
> 
> Only question is if we need to land this as part of this sec bug, or should
> we land that in a separate open bug?

This logging was helpful in determining what was going on, so I'm just going to leave it here for simplicity.

Byron Campen [:bwc]

Assignee

Comment 24

•

7 years ago

(In reply to Nils Ohlmeier [:drno] from comment #22)
> Comment on attachment 8934709 [details] [diff] [review]
> Part 2: Do offer/answer validation sooner
> 
> Review of attachment 8934709 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> LGTM
> 
> My only small concern is that you are running ValidateOffer() and
> ValidateAnswer() on our own offers and answer, which I assume is the point
> of this change. Have you done a try run to make sure that we are not
> rejecting our own offers/answers?

This is not the point of this change, but we should be doing it. The point of the change was that a bogus remote answer was getting partially applied before we realized it was bogus and bailed, leaving JsepSessionImpl in a state that violated some invariants. I will be doing a try run, of course.

Byron Campen [:bwc]

Assignee

Comment 25

•

7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e01d1eabb63a

Daniel Veditz [:dveditz]

Updated

•

7 years ago

Flags: sec-bounty?

Al Billings [:abillings - ex-MoCo]

Comment 26

•

7 years ago

Comment on attachment 8934709 [details] [diff] [review]
Part 2: Do offer/answer validation sooner

sec-approval+ for trunk. Please nominate for beta.

Attachment #8934709 - Flags: sec-approval? → sec-approval+

Byron Campen [:bwc]

Assignee

Comment 27

•

7 years ago

I will create some patches to apply to beta.

Byron Campen [:bwc]

Assignee

Comment 28

•

7 years ago

FYI, the test-cases do not seem to cause crashes on release (linux ASAN), and probably won't be much different on beta.

Byron Campen [:bwc]

Assignee

Comment 29

•

7 years ago

Attached patch (beta backport) Do offer/answer validation sooner — Details — Splinter Review

Byron Campen [:bwc]

Assignee

Comment 30

•

7 years ago

Comment on attachment 8935060 [details] [diff] [review]
(beta backport) Do offer/answer validation sooner

Approval Request Comment
[Feature/Bug causing the regression]:
   See sec-approval.

[User impact if declined]:
   Possible crash vulnerabilities, although none have been discovered.

[Is this code covered by automated tests?]:
   Yes.

[Has the fix been verified in Nightly?]:
   Not yet, although try looks good.

[Needs manual test from QE? If yes, steps to reproduce]: 
   No.

[List of other uplifts needed for the feature/fix]:
   None.

[Is the change risky?]:
   Not very.

[Why is the change risky/not risky?]:
   We are moving some validation a little bit sooner, there isn't really any new code in here.

[String changes made/needed]:
   None.

Attachment #8935060 - Flags: approval-mozilla-beta?

Byron Campen [:bwc]

Assignee

Updated

•

7 years ago

Flags: needinfo?(loobenyang)

Byron Campen [:bwc]

Assignee

Comment 31

•

7 years ago

Do the binaries in the try push in comment 25 withstand the test-cases for you?

Flags: needinfo?(loobenyang)

Looben Yang

Reporter

Comment 32

•

7 years ago

(In reply to Byron Campen [:bwc] from comment #31)
> Do the binaries in the try push in comment 25 withstand the test-cases for
> you?

Could you paste a direct link to the build?
I navigate into this link but could not see any build except all kinds of test results:

Job(sig) : Windows 7 opt Executed by TaskCluster test-windows7-32/opt-cppunit tc(Cpp)
Machine: i-0ffd47c719cdc9bce
Task:NFUcIARCTD2IraY5DATYFg
Build:- windows7-32 -
Job name:test-windows7-32/opt-cppunit
Requested:Thu Dec 7, 04:47:23

Flags: needinfo?(loobenyang)

Byron Campen [:bwc]

Assignee

Comment 33

•

7 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c5917dbeca0aa774e996b4d4d2bd7f3f9cc2dc0
Bug 1422215 - Part 1: Make logging more consistent in JsepSessionImpl. r=drno

https://hg.mozilla.org/integration/mozilla-inbound/rev/174f97c89a9f6b3cf51216e89a3bd22ab74e049c
Bug 1422215 - Part 2: Do offer/answer validation sooner. r=drno

Ryan VanderMeulen [:RyanVM]

Comment 34

•

7 years ago

Linux64 ASAN: https://queue.taskcluster.net/v1/task/SKB0PRdQQueH0ITdMF73Mg/runs/0/artifacts/public/build/target.tar.bz2

Win32 opt: https://queue.taskcluster.net/v1/task/ENcciua-TDiR84FUe9gbWg/runs/0/artifacts/public/build/target.zip

Win64 opt: https://queue.taskcluster.net/v1/task/GbWxCJYlSPyML8ejdv19-Q/runs/0/artifacts/public/build/target.zip

Hope that helps :)

Byron Campen [:bwc]

Assignee

Comment 35

•

7 years ago

(In reply to Looben Yang from comment #32)
> (In reply to Byron Campen [:bwc] from comment #31)
> > Do the binaries in the try push in comment 25 withstand the test-cases for
> > you?
> 
> Could you paste a direct link to the build?
> I navigate into this link but could not see any build except all kinds of
> test results:
> 
> Job(sig) : Windows 7 opt Executed by TaskCluster
> test-windows7-32/opt-cppunit tc(Cpp)
> Machine: i-0ffd47c719cdc9bce
> Task:NFUcIARCTD2IraY5DATYFg
> Build:- windows7-32 -
> Job name:test-windows7-32/opt-cppunit
> Requested:Thu Dec 7, 04:47:23

What you're looking for is a green 'B' at the beginning of a line; right now we do four windows builds; Windows 2012 pgo, Windows 2012 x64 pgo, Windows 2012 NoOpt debug, and Windows 2012 x64 NoOpt debug. Pick whichever one of those four you want, and click on it. You'll see the installer in the list of artifacts.

Or, you could wait until tomorrow and try nightly (provided the landing doesn't get backed out for some reason).

Looben Yang

Reporter

Comment 36

•

7 years ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #34)
> Linux64 ASAN:
> https://queue.taskcluster.net/v1/task/SKB0PRdQQueH0ITdMF73Mg/runs/0/
> artifacts/public/build/target.tar.bz2
> 
> Win32 opt:
> https://queue.taskcluster.net/v1/task/ENcciua-TDiR84FUe9gbWg/runs/0/
> artifacts/public/build/target.zip
> 
> Win64 opt:
> https://queue.taskcluster.net/v1/task/GbWxCJYlSPyML8ejdv19-Q/runs/0/
> artifacts/public/build/target.zip
> 
> Hope that helps :)

Thanks Ryan. It does help. 

Just tried with the Win32 opt build above, could not reproduce it.
Thanks Byron for the quick fix.

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 37

•

7 years ago

https://hg.mozilla.org/mozilla-central/rev/4c5917dbeca0
https://hg.mozilla.org/mozilla-central/rev/174f97c89a9f

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox59: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla59

Julien Cristau [:jcristau]

Comment 38

•

7 years ago

Comment on attachment 8935060 [details] [diff] [review]
(beta backport) Do offer/answer validation sooner

sec-crit, new regression in 58, beta58+

Attachment #8935060 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Ryan VanderMeulen [:RyanVM]

Comment 39

•

7 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/1cc170b6689a

status-firefox58: affected → fixed

Daniel Veditz [:dveditz]

Updated

•

6 years ago

Group: media-core-security → core-security-release

Daniel Veditz [:dveditz]

Updated

•

6 years ago

Flags: needinfo?(dveditz)

Daniel Veditz [:dveditz]

Updated

•

6 years ago

Flags: sec-bounty?

Flags: sec-bounty+

Flags: needinfo?(dveditz)

Matt Wobensmith [:mwobensmith][:matt:]

Updated

•

6 years ago

Flags: qe-verify+

Whiteboard: [post-critsmash-triage]

Bogdan Maris, Desktop QA

Comment 40

•

6 years ago

I was able to reproduce this issue with the information from comment 15, on Ubuntu 16.04. I retested with the Latest Nightly 59.0a1 and latest Beta 58.0b18 asan build, and the issue has no longer appeared.

Status: RESOLVED → VERIFIED

status-firefox58: fixed → verified

status-firefox59: fixed → verified

Flags: qe-verify+

Daniel Veditz [:dveditz]

Updated

•

6 years ago

Group: core-security-release

UAF_CheckNegotiationNeeded_PoC.html 7 years ago Looben Yang 2.13 KB, text/plain		Details
UAF_CheckNegotiationNeeded_PoC_EIP_41414141.js 7 years ago Looben Yang 4.06 KB, text/plain		Details
Attempt to diagnose 7 years ago Byron Campen [:bwc] 1.35 KB, patch		Details \| Diff \| Splinter Review
UAF_CheckNegotiationNeeded_PoC2.html 7 years ago Looben Yang 505.67 KB, text/plain		Details
Part 2: Do offer/answer validation sooner 7 years ago Byron Campen [:bwc] 4.51 KB, patch	drno : review+ abillings : sec-approval+	Details \| Diff \| Splinter Review
Part 1: Make logging more consistent in JsepSessionImpl 7 years ago Byron Campen [:bwc] 7.56 KB, patch	drno : review+	Details \| Diff \| Splinter Review
(beta backport) Do offer/answer validation sooner 7 years ago Byron Campen [:bwc] 5.01 KB, patch	jcristau : approval-mozilla-beta+	Details \| Diff \| Splinter Review