Closed Bug 1595886 Opened 6 years ago Closed 5 years ago

AddressSanitizer: heap-use-after-free z:\build\build\src\js\xpconnect\src\XPCConvert.cpp:106 in XPCConvert::NativeData2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,en

Categories

(Core :: Networking, defect, P2)

Product:
Core
Component:
Networking
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox-esr68 76+ fixed
firefox75 --- wontfix
firefox76 + fixed
firefox77 + fixed

People

(Reporter: malexandru, Assigned: valentin)

References

Details

(Keywords: csectype-uaf, sec-high, Whiteboard: [necko-triaged][post-critsmash-triage][adv-main76+r][adv-ESR68.8+r])

Attachments

(1 file, 1 obsolete file)

Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr68+
dveditz
: sec-approval+
Details | Review

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=275841715&repo=mozilla-esr68&lineNumber=48337

Failure log:

18:07:12     INFO - TEST-START | docshell/test/chrome/test_bug565388.xul
18:07:13     INFO - GECKO(5392) | =================================================================
18:07:13    ERROR - GECKO(5392) | ==7740==ERROR: AddressSanitizer: heap-use-after-free on address 0x12b1a863ee58 at pc 0x7ffb2aad2023 bp 0x00d13b9fbe20 sp 0x00d13b9fbe68
18:07:13     INFO - GECKO(5392) | READ of size 1 at 0x12b1a863ee58 thread T0
18:07:13     INFO - GECKO(5392) |     #0 0x7ffb2aad2022 in XPCConvert::NativeData2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,enum nsresult *) z:\build\build\src\js\xpconnect\src\XPCConvert.cpp:106
18:07:13     INFO - GECKO(5392) |     #1 0x7ffb2aad41e8 in XPCConvert::NativeArray2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,enum nsresult *) z:\build\build\src\js\xpconnect\src\XPCConvert.cpp:1416
18:07:13     INFO - GECKO(5392) |     #2 0x7ffb2aad0e13 in XPCConvert::NativeData2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,enum nsresult *) z:\build\build\src\js\xpconnect\src\XPCConvert.cpp
18:07:13     INFO - GECKO(5392) |     #3 0x7ffb2ab5bed6 in nsXPCWrappedJS::CallMethod(unsigned short,struct nsXPTMethodInfo const *,struct nsXPTCMiniVariant *) z:\build\build\src\js\xpconnect\src\XPCWrappedJSClass.cpp:917
18:07:13     INFO - GECKO(5392) |     #4 0x7ffb28dd193a in PrepareAndDispatch z:\build\build\src\xpcom\reflect\xptcall\md\win32\xptcstubs_x86_64.cpp:181
18:07:14     INFO - GECKO(5392) |     #5 0x7ffb39bc36b8 in SharedStub (Z:\task_1573580361\build\application\firefox\xul.dll+0x1911236b8)
18:07:14     INFO - GECKO(5392) |     #6 0x7ffb29043bfb in NS_SniffContent(char const *,class nsIRequest *,unsigned char const *,unsigned int,class nsTSubstring<char> &) z:\build\build\src\netwerk\base\nsNetUtil.cpp:2645
18:07:14     INFO - GECKO(5392) |     #7 0x7ffb28fab77a in CallTypeSniffers z:\build\build\src\netwerk\base\nsBaseChannel.cpp:758
18:07:14     INFO - GECKO(5392) |     #8 0x7ffb28ff785c in CallPeekFunc z:\build\build\src\netwerk\base\nsInputStreamPump.cpp:85
18:07:14     INFO - GECKO(5392) |     #9 0x7ffb28cfb9bb in nsStringInputStream::ReadSegments(enum nsresult (*)(class nsIInputStream *,void *,char const *,unsigned int,unsigned int,unsigned int *),void *,unsigned int,unsigned int *) z:\build\build\src\xpcom\io\nsStringStream.cpp:300
18:07:14     INFO - GECKO(5392) |     #10 0x7ffb28c948f6 in mozilla::NonBlockingAsyncInputStream::ReadSegments(enum nsresult (*)(class nsIInputStream *,void *,char const *,unsigned int,unsigned int,unsigned int *),void *,unsigned int,unsigned int *) z:\build\build\src\xpcom\io\NonBlockingAsyncInputStream.cpp:226
18:07:14     INFO - GECKO(5392) |     #11 0x7ffb28ff725a in nsInputStreamPump::PeekStream(void (*)(void *,unsigned char const *,unsigned int),void *) z:\build\build\src\netwerk\base\nsInputStreamPump.cpp:105
18:07:14     INFO - GECKO(5392) |     #12 0x7ffb28faaf76 in nsBaseChannel::OnStartRequest(class nsIRequest *) z:\build\build\src\netwerk\base\nsBaseChannel.cpp:790
18:07:14     INFO - GECKO(5392) |     #13 0x7ffb28ffa5bf in nsInputStreamPump::OnStateStart(void) z:\build\build\src\netwerk\base\nsInputStreamPump.cpp:487
18:07:14     INFO - GECKO(5392) |     #14 0x7ffb28ff9ad4 in nsInputStreamPump::OnInputStreamReady(class nsIAsyncInputStream *) z:\build\build\src\netwerk\base\nsInputStreamPump.cpp:396
18:07:14     INFO - GECKO(5392) |     #15 0x7ffb28c94543 in mozilla::NonBlockingAsyncInputStream::AsyncWaitRunnable::Run(void) z:\build\build\src\xpcom\io\NonBlockingAsyncInputStream.cpp:29
18:07:14     INFO - GECKO(5392) |     #16 0x7ffb28da00fb in nsThread::ProcessNextEvent(bool,bool *) z:\build\build\src\xpcom\threads\nsThread.cpp:1175
18:07:14     INFO - GECKO(5392) |     #17 0x7ffb28da76d8 in NS_ProcessNextEvent(class nsIThread *,bool) z:\build\build\src\xpcom\threads\nsThreadUtils.cpp:486
18:07:14     INFO - GECKO(5392) |     #18 0x7ffb29ef505f in mozilla::ipc::MessagePump::Run(class base::MessagePump::Delegate *) z:\build\build\src\ipc\glue\MessagePump.cpp:88
18:07:14     INFO - GECKO(5392) |     #19 0x7ffb29e4f92e in MessageLoop::RunHandler(void) z:\build\build\src\ipc\chromium\src\base\message_loop.cc:308
18:07:14     INFO - GECKO(5392) |     #20 0x7ffb29e4f6c5 in MessageLoop::Run(void) z:\build\build\src\ipc\chromium\src\base\message_loop.cc:290
18:07:14     INFO - GECKO(5392) |     #21 0x7ffb32fae24a in nsBaseAppShell::Run(void) z:\build\build\src\widget\nsBaseAppShell.cpp:137
18:07:14     INFO - GECKO(5392) |     #22 0x7ffb331410e9 in nsAppShell::Run(void) z:\build\build\src\widget\windows\nsAppShell.cpp:412
18:07:14     INFO - GECKO(5392) |     #23 0x7ffb36e8ecc8 in nsAppStartup::Run(void) z:\build\build\src\toolkit\components\startup\nsAppStartup.cpp:276
18:07:14     INFO - GECKO(5392) |     #24 0x7ffb371b5d18 in XREMain::XRE_mainRun(void) z:\build\build\src\toolkit\xre\nsAppRunner.cpp:4616
18:07:14     INFO - GECKO(5392) |     #25 0x7ffb371b9d90 in XREMain::XRE_main(int,char * * const,struct mozilla::BootstrapConfig const &) z:\build\build\src\toolkit\xre\nsAppRunner.cpp:4750
18:07:14     INFO - GECKO(5392) |     #26 0x7ffb371bbc1e in XRE_main(int,char * * const,struct mozilla::BootstrapConfig const &) z:\build\build\src\toolkit\xre\nsAppRunner.cpp:4831
18:07:14     INFO - GECKO(5392) |     #27 0x7ff67b6821ca in NS_internal_main(int,char * *,char * *) z:\build\build\src\browser\app\nsBrowserApp.cpp:296
18:07:14     INFO - GECKO(5392) |     #28 0x7ff67b6814f2 in wmain z:\build\build\src\toolkit\xre\nsWindowsWMain.cpp:131
18:07:14     INFO - GECKO(5392) |     #29 0x7ff67b767a57 in __scrt_common_main_seh f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:288
18:07:14     INFO - GECKO(5392) |     #30 0x7ffb7ad53033  (C:\Windows\System32\KERNEL32.DLL+0x180013033)
18:07:14     INFO - GECKO(5392) |     #31 0x7ffb7d041460  (C:\Windows\SYSTEM32\ntdll.dll+0x180071460)
18:07:14     INFO - GECKO(5392) | 0x12b1a863ee58 is located 8 bytes inside of 32-byte region [0x12b1a863ee50,0x12b1a863ee70)
18:07:14     INFO - GECKO(5392) | freed by thread T0 here:
18:07:14     INFO - GECKO(5392) |     #0 0x7ffb547644e0 in free Z:\task_1573488744\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cc:53
18:07:14     INFO - GECKO(5392) |     #1 0x7ffb28b3900a in nsTSubstring<char>::SetIsVoid(bool) z:\build\build\src\xpcom\string\nsTSubstring.cpp:969
18:07:14     INFO - GECKO(5392) |     #2 0x7ffb28cfb59b in nsStringInputStream::Close(void) z:\build\build\src\xpcom\io\nsStringStream.cpp:254
18:07:14     INFO - GECKO(5392) |     #3 0x7ffb28c93c55 in mozilla::NonBlockingAsyncInputStream::Close(void) z:\build\build\src\xpcom\io\NonBlockingAsyncInputStream.cpp:142
18:07:14     INFO - GECKO(5392) |     #4 0x7ffb28ff8183 in nsInputStreamPump::Cancel(enum nsresult) z:\build\build\src\netwerk\base\nsInputStreamPump.cpp:196
18:07:14     INFO - GECKO(5392) |     #5 0x7ffb28fa8136 in nsBaseChannel::Cancel(enum nsresult) z:\build\build\src\netwerk\base\nsBaseChannel.cpp:392
18:07:14     INFO - GECKO(5392) |     #6 0x7ffb28ffdf71 in mozilla::net::nsLoadGroup::Cancel(enum nsresult) z:\build\build\src\netwerk\base\nsLoadGroup.cpp:224
18:07:14     INFO - GECKO(5392) |     #7 0x7ffb2b1652df in nsDocLoader::Stop(void) z:\build\build\src\uriloader\base\nsDocLoader.cpp:229
18:07:14     INFO - GECKO(5392) |     #8 0x7ffb3642c53d in nsDocShell::Stop(unsigned int) z:\build\build\src\docshell\base\nsDocShell.cpp:4740
18:07:14     INFO - GECKO(5392) |     #9 0x7ffb3643f13c in nsDocShell::Destroy(void) z:\build\build\src\docshell\base\nsDocShell.cpp:5011
18:07:14     INFO - GECKO(5392) |     #10 0x7ffb36af2fa4 in nsWebBrowser::SetDocShell(class nsIDocShell *) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:1238
18:07:14     INFO - GECKO(5392) |     #11 0x7ffb36af0dd4 in nsWebBrowser::InternalDestroy(void) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:192
18:07:14     INFO - GECKO(5392) |     #12 0x7ffb36af955c in nsWebBrowser::Destroy(void) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:909
18:07:14     INFO - GECKO(5392) |     #13 0x7ffb365788e7 in BrowserDestroyer::Run(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:375
18:07:14     INFO - GECKO(5392) |     #14 0x7ffb2c6716c8 in nsContentUtils::AddScriptRunner(class nsIRunnable *) z:\build\build\src\dom\base\nsContentUtils.cpp:5247
18:07:14     INFO - GECKO(5392) |     #15 0x7ffb365784c5 in WindowlessBrowser::~WindowlessBrowser(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:417
18:07:14     INFO - GECKO(5392) |     #16 0x7ffb3657826f in WindowlessBrowser::`scalar deleting destructor'(unsigned int) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:405
18:07:14     INFO - GECKO(5392) |     #17 0x7ffb36528892 in WindowlessBrowser::Release(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:430
18:07:14     INFO - GECKO(5392) |     #18 0x7ffb28b90833 in mozilla::SegmentedVector<class nsCOMPtr<class nsISupports>,4096,class mozilla::MallocAllocPolicy>::PopLastN(unsigned int) z:\build\build\src\obj-firefox\dist\include\mozilla\SegmentedVector.h:235
18:07:14     INFO - GECKO(5392) |     #19 0x7ffb28b6fb72 in mozilla::dom::DeferredFinalizerImpl<class nsISupports>::DeferredFinalize(unsigned int,void *) z:\build\build\src\obj-firefox\dist\include\mozilla\dom\BindingUtils.h:2713
18:07:14     INFO - GECKO(5392) |     #20 0x7ffb28b707c1 in mozilla::IncrementalFinalizeRunnable::ReleaseNow(bool) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1279
18:07:14     INFO - GECKO(5392) |     #21 0x7ffb28b714f8 in mozilla::CycleCollectedJSRuntime::FinalizeDeferredThings(enum mozilla::CycleCollectedJSContext::DeferredFinalizeType) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1359
18:07:14     INFO - GECKO(5392) |     #22 0x7ffb28b6b7e5 in mozilla::CycleCollectedJSRuntime::OnGC(struct JSContext *,enum JSGCStatus) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1418
18:07:14     INFO - GECKO(5392) |     #23 0x7ffb382305b8 in js::gc::GCRuntime::maybeCallGCCallback(enum JSGCStatus) z:\build\build\src\js\src\gc\GC.cpp:7408
18:07:14     INFO - GECKO(5392) |     #24 0x7ffb38231706 in js::gc::GCRuntime::gcCycle(bool,class js::SliceBudget,enum JS::GCReason) z:\build\build\src\js\src\gc\GC.cpp:7497
18:07:14     INFO - GECKO(5392) |     #25 0x7ffb382348a6 in js::gc::GCRuntime::collect(bool,class js::SliceBudget,enum JS::GCReason) z:\build\build\src\js\src\gc\GC.cpp:7655
18:07:14     INFO - GECKO(5392) |     #26 0x7ffb38269a44 in js::AllocateObject<1>(struct JSContext *,enum js::gc::AllocKind,unsigned __int64,enum js::gc::InitialHeap,struct js::Class const *) z:\build\build\src\js\src\gc\Allocator.cpp:59
18:07:14     INFO - GECKO(5392) |     #27 0x7ffb3756e40d in js::ArrayObject::createArrayInternal(struct JSContext *,enum js::gc::AllocKind,enum js::gc::InitialHeap,class JS::Handle<class js::Shape *>,class JS::Handle<class js::ObjectGroup *>,class js::AutoSetNewObjectMetadata &) z:\build\build\src\js\src\vm\ArrayObject-inl.h:54
18:07:14     INFO - GECKO(5392) |     #28 0x7ffb3750eee6 in js::NewDenseFullyAllocatedArray(struct JSContext *,unsigned int,class JS::Handle<class JSObject *>,enum js::NewObjectKind) z:\build\build\src\js\src\builtin\Array.cpp:4123
18:07:14     INFO - GECKO(5392) |     #29 0x7ffb2aad3fde in XPCConvert::NativeArray2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,enum nsresult *) z:\build\build\src\js\xpconnect\src\XPCConvert.cpp:1405
18:07:14     INFO - GECKO(5392) | previously allocated by thread T0 here:
18:07:14     INFO - GECKO(5392) |     #0 0x7ffb547645d0 in malloc Z:\task_1573488744\build\src\build\build-clang\build-clang\src\llvm\projects\compiler-rt\lib\asan\asan_malloc_win.cc:69
18:07:14     INFO - GECKO(5392) |     #1 0x7ffb28b23501 in nsTSubstring<char>::StartBulkWriteImpl(unsigned int,unsigned int,bool,unsigned int,unsigned int,unsigned int) z:\build\build\src\xpcom\string\nsTSubstring.cpp:203
18:07:14     INFO - GECKO(5392) |     #2 0x7ffb28b3735b in nsTSubstring<char>::Assign(char const *,unsigned int,struct std::nothrow_t const &) z:\build\build\src\xpcom\string\nsTSubstring.cpp:408
18:07:14     INFO - GECKO(5392) |     #3 0x7ffb28cfabed in nsStringInputStream::SetData(class nsTSubstring<char> const &) z:\build\build\src\xpcom\io\nsStringStream.cpp:161
18:07:14     INFO - GECKO(5392) |     #4 0x7ffb28cfdd04 in NS_NewCStringInputStream(class nsIInputStream * *,class nsTSubstring<char> const &) z:\build\build\src\xpcom\io\nsStringStream.cpp:539
18:07:14     INFO - GECKO(5392) |     #5 0x7ffb2981e540 in nsDataChannel::OpenContentStream(bool,class nsIInputStream * *,class nsIChannel * *) z:\build\build\src\netwerk\protocol\data\nsDataChannel.cpp:97
18:07:14     INFO - GECKO(5392) |     #6 0x7ffb28fa661d in nsBaseChannel::BeginPumpingData(void) z:\build\build\src\netwerk\base\nsBaseChannel.cpp:232
18:07:14     INFO - GECKO(5392) |     #7 0x7ffb28f91be3 in nsBaseChannel::AsyncOpen(class nsIStreamListener *) z:\build\build\src\netwerk\base\nsBaseChannel.cpp:678
18:07:14     INFO - GECKO(5392) |     #8 0x7ffb2b17c963 in nsURILoader::OpenURI(class nsIChannel *,unsigned int,class nsIInterfaceRequestor *) z:\build\build\src\uriloader\base\nsURILoader.cpp:847
18:07:14     INFO - GECKO(5392) |     #9 0x7ffb364bdbbe in nsDocShell::OpenInitializedChannel(class nsIChannel *,class nsIURILoader *,unsigned int) z:\build\build\src\docshell\base\nsDocShell.cpp:10627
18:07:14     INFO - GECKO(5392) |     #10 0x7ffb364bf465 in nsDocShell::DoChannelLoad(class nsIChannel *,class nsIURILoader *,bool) z:\build\build\src\docshell\base\nsDocShell.cpp:10599
18:07:14     INFO - GECKO(5392) |     #11 0x7ffb364b9f71 in nsDocShell::DoURILoad(class nsDocShellLoadState *,bool,class nsIDocShell * *,class nsIRequest * *) z:\build\build\src\docshell\base\nsDocShell.cpp:10410
18:07:14     INFO - GECKO(5392) |     #12 0x7ffb3644e700 in nsDocShell::InternalLoad(class nsDocShellLoadState *,class nsIDocShell * *,class nsIRequest * *) z:\build\build\src\docshell\base\nsDocShell.cpp:9680
18:07:14     INFO - GECKO(5392) |     #13 0x7ffb36447aa5 in nsDocShell::LoadURI(class nsDocShellLoadState *) z:\build\build\src\docshell\base\nsDocShell.cpp:770
18:07:14     INFO - GECKO(5392) |     #14 0x7ffb2caa5af8 in mozilla::dom::Location::SetURI(class nsIURI *,class nsIPrincipal &,class mozilla::ErrorResult &,bool) z:\build\build\src\dom\base\Location.cpp:237
18:07:14     INFO - GECKO(5392) |     #15 0x7ffb2caaa72e in mozilla::dom::Location::SetHrefWithBase(class nsTSubstring<UNKNOWN> const &,class nsIURI *,class nsIPrincipal &,bool,class mozilla::ErrorResult &) z:\build\build\src\dom\base\Location.cpp:468
18:07:14     INFO - GECKO(5392) |     #16 0x7ffb2caa9cb2 in mozilla::dom::Location::SetHref(class nsTSubstring<UNKNOWN> const &,class nsIPrincipal &,class mozilla::ErrorResult &) z:\build\build\src\dom\base\Location.cpp:414
18:07:14     INFO - GECKO(5392) |     #17 0x7ffb2d8763f9 in mozilla::dom::Location_Binding::set_href z:\build\build\src\obj-firefox\dom\bindings\LocationBinding.cpp:161
18:07:14     INFO - GECKO(5392) |     #18 0x7ffb3046225d in mozilla::dom::binding_detail::GenericSetter<struct mozilla::dom::binding_detail::CrossOriginThisPolicy>(struct JSContext *,unsigned int,union JS::Value *) z:\build\build\src\dom\bindings\BindingUtils.cpp:3186
18:07:14     INFO - GECKO(5392) |     #19 0x7ffb374950e2 in js::InternalCallOrConstruct(struct JSContext *,class JS::CallArgs const &,enum js::MaybeConstruct) z:\build\build\src\js\src\vm\Interpreter.cpp:535
18:07:14     INFO - GECKO(5392) |     #20 0x7ffb3749a8cf in js::CallSetter(struct JSContext *,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>) z:\build\build\src\js\src\vm\Interpreter.cpp:744
18:07:14     INFO - GECKO(5392) |     #21 0x7ffb3815db68 in js::SetPropertyIgnoringNamedGetter(struct JSContext *,class JS::Handle<class JSObject *>,class JS::Handle<struct JS::PropertyKey>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::Handle<struct JS::PropertyDescriptor>,class JS::ObjectOpResult &) z:\build\build\src\js\src\proxy\BaseProxyHandler.cpp:225
18:07:14     INFO - GECKO(5392) |     #22 0x7ffb3043f17b in mozilla::dom::DOMProxyHandler::set(struct JSContext *,class JS::Handle<class JSObject *>,class JS::Handle<struct JS::PropertyKey>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::ObjectOpResult &)const  z:\build\build\src\dom\bindings\DOMJSProxyHandler.cpp:242
18:07:14     INFO - GECKO(5392) |     #23 0x7ffb2d7dc20f in mozilla::dom::Location_Binding::DOMProxyHandler::set(struct JSContext *,class JS::Handle<class JSObject *>,class JS::Handle<struct JS::PropertyKey>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::ObjectOpResult &)const  z:\build\build\src\obj-firefox\dom\bindings\LocationBinding.cpp:1671
18:07:14     INFO - GECKO(5392) |     #24 0x7ffb38195189 in js::Proxy::set(struct JSContext *,class JS::Handle<class JSObject *>,class JS::Handle<struct JS::PropertyKey>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::ObjectOpResult &) z:\build\build\src\js\src\proxy\Proxy.cpp:403
18:07:14     INFO - GECKO(5392) |     #25 0x7ffb3808dfa5 in JS_SetProperty(struct JSContext *,class JS::Handle<class JSObject *>,char const *,class JS::Handle<union JS::Value>) z:\build\build\src\js\src\jsapi.cpp:2443
18:07:14     INFO - GECKO(5392) |     #26 0x7ffb2f1998c4 in mozilla::dom::Window_Binding::set_location z:\build\build\src\obj-firefox\dom\bindings\WindowBinding.cpp:1463
18:07:14     INFO - GECKO(5392) |     #27 0x7ffb3046225d in mozilla::dom::binding_detail::GenericSetter<struct mozilla::dom::binding_detail::CrossOriginThisPolicy>(struct JSContext *,unsigned int,union JS::Value *) z:\build\build\src\dom\bindings\BindingUtils.cpp:3186
18:07:14     INFO - GECKO(5392) |     #28 0x7ffb374950e2 in js::InternalCallOrConstruct(struct JSContext *,class JS::CallArgs const &,enum js::MaybeConstruct) z:\build\build\src\js\src\vm\Interpreter.cpp:535
18:07:14     INFO - GECKO(5392) |     #29 0x7ffb3749a8cf in js::CallSetter(struct JSContext *,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>,class JS::Handle<union JS::Value>) z:\build\build\src\js\src\vm\Interpreter.cpp:744
18:07:14     INFO - GECKO(5392) | SUMMARY: AddressSanitizer: heap-use-after-free z:\build\build\src\js\xpconnect\src\XPCConvert.cpp:106 in XPCConvert::NativeData2JS(struct JSContext *,class JS::MutableHandle<union JS::Value>,void const *,struct nsXPTType const &,struct nsID const *,unsigned int,enum nsresult *)
18:07:14     INFO - GECKO(5392) | Shadow bytes around the buggy address:
18:07:14     INFO - GECKO(5392) |   0x0501dccc7d70: 00 00 00 00 fa fa fd fd fd fa fa fa fd fd fd fa
18:07:14     INFO - GECKO(5392) |   0x0501dccc7d80: fa fa 00 00 00 fa fa fa fd fd fd fa fa fa fd fd
18:07:14     INFO - GECKO(5392) |   0x0501dccc7d90: fd fd fa fa fd fd fd fd fa fa fd fd fd fa fa fa
18:07:14     INFO - GECKO(5392) |   0x0501dccc7da0: 00 00 00 fa fa fa 00 00 00 00 fa fa 00 00 00 00
18:07:14     INFO - GECKO(5392) |   0x0501dccc7db0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
18:07:14     INFO - GECKO(5392) | =>0x0501dccc7dc0: fd fa fa fa fd fd fd fd fa fa fd[fd]fd fd fa fa
18:07:14     INFO - GECKO(5392) |   0x0501dccc7dd0: 00 00 00 fa fa fa fd fd fd fa fa fa 00 00 00 00
18:07:14     INFO - GECKO(5392) |   0x0501dccc7de0: fa fa 00 00 00 00 fa fa fd fd fd fa fa fa 00 00
18:07:14     INFO - GECKO(5392) |   0x0501dccc7df0: 00 fa fa fa fd fd fd fd fa fa fd fd fd fd fa fa
18:07:14     INFO - GECKO(5392) |   0x0501dccc7e00: 00 00 00 00 fa fa fd fd fd fa fa fa 00 00 00 fa
18:07:14     INFO - GECKO(5392) |   0x0501dccc7e10: fa fa 00 00 00 00 fa fa fd fd fd fd fa fa fd fd
18:07:14     INFO - GECKO(5392) | Shadow byte legend (one shadow byte represents 8 application bytes):
18:07:14     INFO - GECKO(5392) |   Addressable:           00
18:07:14     INFO - GECKO(5392) |   Partially addressable: 01 02 03 04 05 06 07
18:07:14     INFO - GECKO(5392) |   Heap left redzone:       fa
18:07:14     INFO - GECKO(5392) |   Freed heap region:       fd
18:07:14     INFO - GECKO(5392) |   Stack left redzone:      f1
18:07:14     INFO - GECKO(5392) |   Stack mid redzone:       f2
18:07:14     INFO - GECKO(5392) |   Stack right redzone:     f3
18:07:14     INFO - GECKO(5392) |   Stack after return:      f5
18:07:14     INFO - GECKO(5392) |   Stack use after scope:   f8
18:07:14     INFO - GECKO(5392) |   Global redzone:          f9
18:07:14     INFO - GECKO(5392) |   Global init order:       f6
18:07:14     INFO - GECKO(5392) |   Poisoned by user:        f7
18:07:14     INFO - GECKO(5392) |   Container overflow:      fc
18:07:14     INFO - GECKO(5392) |   Array cookie:            ac
18:07:14     INFO - GECKO(5392) |   Intra object redzone:    bb
18:07:14     INFO - GECKO(5392) |   ASan internal:           fe
18:07:14     INFO - GECKO(5392) |   Left alloca redzone:     ca
18:07:14     INFO - GECKO(5392) |   Right alloca redzone:    cb
18:07:14     INFO - GECKO(5392) |   Shadow gap:              cc
18:07:14     INFO - GECKO(5392) | ==7740==ABORTING
18:07:14     INFO - TEST-INFO | Main app process: exit 1
18:07:14     INFO - Buffered messages finished
18:07:14    ERROR - TEST-UNEXPECTED-FAIL | docshell/test/chrome/test_bug565388.xul | application terminated with exit code 1
18:07:14     INFO - runtests.py | Application ran for: 0:00:30.616000
18:07:14     INFO - zombiecheck | Reading PID log: c:\users\task_1573580361\appdata\local\temp\tmprv_blfpidlog
18:07:14     INFO - Stopping web server
18:07:14     INFO - Stopping web socket server
18:07:14     INFO - Stopping ssltunnel
18:07:14  WARNING - leakcheck | refcount logging is off, so leaks can't be detected!
18:07:14     INFO - runtests.py | Running tests: end.
18:07:14     INFO - Buffered messages finished
18:07:14     INFO - Running manifest: dom\animation\test\chrome.ini
18:07:14     INFO - The following extra prefs will be set:
18:07:14     INFO -   dom.animations-api.compositing.enabled=true
18:07:14     INFO -   gfx.omta.background-color=true
18:07:14     INFO -   layout.css.individual-transform.enabled=true

The top few frames of the use are in XPConnect, but it looks like what is really happening is that we're passing in a dead string to something implemented in JS, on this line in NS_SniffContent:
nsresult rv = sniffers[i]->GetMIMETypeFromContent(aRequest, aData, aLength, aSniffedType);

So I think this is really a lifetime issue for a string associated with a string input stream.

Group: core-security → network-core-security
Component: XPConnect → Networking
Keywords: csectype-uaf, sec-high

Junior, could you have a look? Thanks!

Flags: needinfo?(juhsu)

The UAF happens in reading the data stream, at this moment, the docshell shouldn't be garbage recycled even it's invisible.

18:07:14     INFO - GECKO(5392) |     #9 0x7ffb3643f13c in nsDocShell::Destroy(void) z:\build\build\src\docshell\base\nsDocShell.cpp:5011
18:07:14     INFO - GECKO(5392) |     #10 0x7ffb36af2fa4 in nsWebBrowser::SetDocShell(class nsIDocShell *) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:1238
18:07:14     INFO - GECKO(5392) |     #11 0x7ffb36af0dd4 in nsWebBrowser::InternalDestroy(void) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:192
18:07:14     INFO - GECKO(5392) |     #12 0x7ffb36af955c in nsWebBrowser::Destroy(void) z:\build\build\src\toolkit\components\browser\nsWebBrowser.cpp:909
18:07:14     INFO - GECKO(5392) |     #13 0x7ffb365788e7 in BrowserDestroyer::Run(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:375
18:07:14     INFO - GECKO(5392) |     #14 0x7ffb2c6716c8 in nsContentUtils::AddScriptRunner(class nsIRunnable *) z:\build\build\src\dom\base\nsContentUtils.cpp:5247
18:07:14     INFO - GECKO(5392) |     #15 0x7ffb365784c5 in WindowlessBrowser::~WindowlessBrowser(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:417
18:07:14     INFO - GECKO(5392) |     #16 0x7ffb3657826f in WindowlessBrowser::`scalar deleting destructor'(unsigned int) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:405
18:07:14     INFO - GECKO(5392) |     #17 0x7ffb36528892 in WindowlessBrowser::Release(void) z:\build\build\src\xpfe\appshell\nsAppShellService.cpp:430
18:07:14     INFO - GECKO(5392) |     #18 0x7ffb28b90833 in mozilla::SegmentedVector<class nsCOMPtr<class nsISupports>,4096,class mozilla::MallocAllocPolicy>::PopLastN(unsigned int) z:\build\build\src\obj-firefox\dist\include\mozilla\SegmentedVector.h:235
18:07:14     INFO - GECKO(5392) |     #19 0x7ffb28b6fb72 in mozilla::dom::DeferredFinalizerImpl<class nsISupports>::DeferredFinalize(unsigned int,void *) z:\build\build\src\obj-firefox\dist\include\mozilla\dom\BindingUtils.h:2713
18:07:14     INFO - GECKO(5392) |     #20 0x7ffb28b707c1 in mozilla::IncrementalFinalizeRunnable::ReleaseNow(bool) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1279
18:07:14     INFO - GECKO(5392) |     #21 0x7ffb28b714f8 in mozilla::CycleCollectedJSRuntime::FinalizeDeferredThings(enum mozilla::CycleCollectedJSContext::DeferredFinalizeType) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1359
18:07:14     INFO - GECKO(5392) |     #22 0x7ffb28b6b7e5 in mozilla::CycleCollectedJSRuntime::OnGC(struct JSContext *,enum JSGCStatus) z:\build\build\src\xpcom\base\CycleCollectedJSRuntime.cpp:1418
Component: Networking → Window Management
Flags: needinfo?(juhsu)

The priority flag is not set for this bug.
:enndeakin, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(enndeakin)
Group: network-core-security → firefox-core-security
Component: Window Management → Headless
Flags: needinfo?(enndeakin)
Product: Core → Firefox

The priority flag is not set for this bug.
:bdahl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(bdahl)

Moving to General, the test fails in non-headless mode.

Component: Headless → General
Flags: needinfo?(bdahl)

(In reply to Brendan Dahl [:bdahl] (away until 30th) from comment #6)

Moving to General, the test fails in non-headless mode.

It's more about the UAF that shows windowless browser code has destroyed the docshell already... Are you saying there's similar UAF failures in non-headless mode? If so, can you provide a link to UAF stacks from there?

Either way I'd like us to stop playing hot potato with this bug - Fx General is approximately the worst place for it...

Flags: needinfo?(bdahl)

Yes, the test runs in non-headless mode and fails. I think there's some confusion because the test creates a WindowlessBrowser which does not mean headless mode. A WindowlessBrowser can be used in headless and non-headless mode.

Flags: needinfo?(bdahl)

https://searchfox.org/mozilla-central/source/xpfe/appshell/nsAppShellService.cpp#344 is where we define windowless browsers, and the bmo metadata says bugs belong in Core :: Window Management.

Neil, please can you take a further look based on Junior's initial investigation? Thanks.

Group: firefox-core-security → dom-core-security
Component: General → Window Management
Flags: needinfo?(enndeakin)
Product: Firefox → Core

I don't know anything about this. The test has been disabled in the meantime. Junior seems to be away right now. Brendan, do you know who might be able to understand what is going on here?

Flags: needinfo?(enndeakin) → needinfo?(bdahl)
Priority: -- → P2

DocShells do get destroyed when they need to be. That isn't about GC or CC, more about when iframes are removed from DOM or when windows are closed.
Does necko not keep the data it is passing to callbacks alive long enough? That would violate the normal COM rules.
As far as I see, this is more a Necko issue.

Component: Window Management → Networking
Flags: needinfo?(bdahl)
Flags: needinfo?(nhnguyen)

Honza, could you take a look at this? See comment 11.

Flags: needinfo?(nhnguyen) → needinfo?(honzab.moz)

This is very weird. nsStringInputStream::ReadSegments checks it has already been closed or not. If yes, it early returns, we should never get to line 300.

The Closed() method should return true (return mData.IsVoid();), because the string has been voided before by cancellation of the channel (Clear has been called, which does mData.SetIsVoid(true);. Both happens on the same thread, so no races.

Could this be some weird XPCOM string bug? nsTSubstring<T>::SetIsVoid and IsVoid() are pretty straightforward, tho.

What is interesting is that nsStringInputStream.mData is nsDependentCSubstring. That usually just refers the data and not making a copy. But we see allocations here, so I'm puzzled.

Flags: needinfo?(honzab.moz)

Note that bug has nothing to do with DocShell lifetime. This is contained in stringinputstream.

What I suspect is happening here is that we have mData, and pass the pointer into JS to sniff the content. The JS triggeres the channel to be closed, which sets mData.SetIsVoid(true); then the JS probably continues to use the pointer it got passed, since it has no way of knowing the memory has been freed.
Since it all happens on the main thread, the RecursiveMutex isn't much help here.
So yeah, it would be nice to pass the data into JS without copying, but since we have no way to later invalidate the pointer, we probably have to copy it to a local buffer.

Group: dom-core-security → network-core-security
Assignee: nobody → valentin.gosu
Whiteboard: [necko-triaged]
Attachment #9135694 - Attachment description: Bug 1595886 - Use a temp string in readSegmments r=dragana,mayhemer → Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

Comment on attachment 9135694 [details]
Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Fairly easily once you figure out what the problem is, and that shouldn't be too hard.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: all
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: applies cleanly on esr68
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely to cause regression (90% confidence)
Attachment #9135694 - Flags: sec-approval?
Attachment #9137973 - Flags: sec-approval?

Comment on attachment 9137973 [details]
Bug 1595886 - Add gtest [DO_NOT_LAND_YET]

As the attachment desc says, "do not land yet". sec-approval not granted. Once the fix is on Release you can land the test without sec-approval and set the in-testsuite flag to +

Attachment #9137973 - Flags: sec-approval? → sec-approval-
Flags: in-testsuite?

(In reply to Daniel Veditz [:dveditz] from comment #19)

Comment on attachment 9137973 [details]
Bug 1595886 - Add gtest [DO_NOT_LAND_YET]

As the attachment desc says, "do not land yet". sec-approval not granted. Once the fix is on Release you can land the test without sec-approval and set the in-testsuite flag to +

Thanks for catching that. I probably forgot to clear the checkbox.

Comment on attachment 9135694 [details]
Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

sec-approval+ for landing on nightly. We'll want this on Beta and ESR too but let's test a week or so before uplift.

Attachment #9135694 - Flags: sec-approval? → sec-approval+
Attachment #9135694 - Attachment description: Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer → Bug 1595886 - Use a temp string in readSegments r=dragana

The Cargo.lock changes failed to apply.

Flags: needinfo?(valentin.gosu)
Attachment #9135694 - Attachment description: Bug 1595886 - Use a temp string in readSegments r=dragana → Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #22)

The Cargo.lock changes failed to apply.

That file was not supposed to be in the patch. Not sure why it gets changed every time I do a build.

Flags: needinfo?(valentin.gosu)

https://hg.mozilla.org/integration/autoland/rev/0c092e04a5a907dc7d6fce1d415215bbd5f33fe7
https://hg.mozilla.org/mozilla-central/rev/0c092e04a5a9

Group: network-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox77: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

The patch landed in nightly and beta is affected.
:valentin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(valentin.gosu)

Comment on attachment 9135694 [details]
Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

Beta/Release Uplift Approval Request

  • User impact if declined: Potential for UAF.
  • Is this code covered by automated tests?: Unknown
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simply makes sure we don't release the string while having a pointer to its contents.
  • String changes made/needed:
Flags: needinfo?(valentin.gosu)
Attachment #9135694 - Flags: approval-mozilla-esr68?
Attachment #9135694 - Flags: approval-mozilla-beta?

Comment on attachment 9135694 [details]
Bug 1595886 - Use a temp string in readSegments r=dragana,mayhemer

Approved for 76.0b7 and 68.8esr.

Attachment #9135694 - Flags: approval-mozilla-esr68?
Attachment #9135694 - Flags: approval-mozilla-esr68+
Attachment #9135694 - Flags: approval-mozilla-beta?
Attachment #9135694 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [necko-triaged] → [necko-triaged][post-critsmash-triage]
Whiteboard: [necko-triaged][post-critsmash-triage] → [necko-triaged][post-critsmash-triage][adv-main76+r]
Whiteboard: [necko-triaged][post-critsmash-triage][adv-main76+r] → [necko-triaged][post-critsmash-triage][adv-main76+r][adv-ESR68.8+r]
Group: core-security-release

Comment on attachment 9137973 [details]
Bug 1595886 - Add gtest [DO_NOT_LAND_YET]

Revision D69509 was moved to bug 1702975. Setting attachment 9137973 [details] to obsolete.

Attachment #9137973 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: