1194562 - Heap-buffer-overflow in js::ExpandErrorArgumentsVA

Status: VERIFIED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Abhishek Arya]

Attachment: Testcase (also create empty.js with no content in same dir)

=================================================================
==21063==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f000032fa0 at pc 0x7f6b7033e963 bp 0x7ffc7de05560 sp 0x7ffc7de05558
READ of size 2 at 0x60f000032fa0 thread T0 (Web Content)
    #0 0x7f6b7033e962 in js_strlen(char16_t const*) js/src/jsstr.cpp:4502:17
    #1 0x7f6b702fde9c in js::ExpandErrorArgumentsVA(js::ExclusiveContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, char**, JSErrorReport*, js::ErrorArgumentsType, __va_list_tag*) js/src/jscntxt.cpp:617:33
    #2 0x7f6b702ffcc1 in ExpandErrorArguments(js::ExclusiveContext*, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, char**, JSErrorReport*, js::ErrorArgumentsType, ...) js/src/jscntxt.cpp:779:21
    #3 0x7f6b702f4deb in js::ReportErrorNumberUCArray(JSContext*, unsigned int, JSErrorFormatString const* (*)(void*, unsigned int), void*, unsigned int, char16_t const**) js/src/jscntxt.cpp:801:10
    #4 0x7f6b6a4852d2 in mozilla::dom::ThrowMethodFailed(JSContext*, mozilla::ErrorResult&) dom/bindings/BindingUtils.cpp:236:3
    #5 0x7f6b697119dc in mozilla::dom::ServiceWorkerContainerBinding::_register__promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::ServiceWorkerContainer*, JSJitMethodCallArgs const&) objdir-ff-asan/dom/bindings/ServiceWorkerContainerBinding.cpp:251:12
    #6 0x7f6b6a49b649 in mozilla::dom::GenericPromiseReturningBindingMethod(JSContext*, unsigned int, JS::Value*) dom/bindings/BindingUtils.cpp:2641:13
    #7 0x7f6b6f705248 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:235:15
    #8 0x7f6b6f74f354 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3035:18
    #9 0x7f6b6f728e35 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:714:12
    #10 0x7f6b6f778a7d in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:955:15
    #11 0x7f6b6f779173 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:988:12
    #12 0x7f6b702ed401 in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::ScopeObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4442:19
    #13 0x7f6b702ee186 in Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4469:12
    #14 0x7f6b6894e3f6 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) dom/base/nsJSUtils.cpp:224:12
    #15 0x7f6b6894f404 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) dom/base/nsJSUtils.cpp:286:10
    #16 0x7f6b689d7134 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) dom/base/nsScriptLoader.cpp:1143:10
    #17 0x7f6b689d41f6 in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) dom/base/nsScriptLoader.cpp:970:10
    #18 0x7f6b689ccfe2 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) dom/base/nsScriptLoader.cpp:764:10
    #19 0x7f6b689c8f87 in nsScriptElement::MaybeProcessScript() dom/base/nsScriptElement.cpp:142:10
    #20 0x7f6b67d18e34 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) dom/base/nsIScriptElement.h:221:18
    #21 0x7f6b67d176fc in nsHtml5TreeOpExecutor::RunFlushLoop() parser/html/nsHtml5TreeOpExecutor.cpp:487:7
    #22 0x7f6b67d1bf2b in nsHtml5ExecutorFlusher::Run() parser/html/nsHtml5StreamParser.cpp:127:9
    #23 0x7f6b6624f1fb in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:864:7
    #24 0x7f6b662ce31c in NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:277:10
    #25 0x7f6b66b9c49e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:95:21
    #26 0x7f6b66b25e91 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:234:3
    #27 0x7f6b6bddf9df in nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:156:3
    #28 0x7f6b6dd70103 in XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:785:12
    #29 0x7f6b66b25e91 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:234:3
    #30 0x7f6b6dd6f627 in XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:621:7
    #31 0x4dbbf4 in content_process_main(int, char**) ipc/contentproc/plugin-container.cpp:237:19
    #32 0x7f6b635b0ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
0x60f000032fa0 is located 0 bytes to the right of 176-byte region [0x60f000032ef0,0x60f000032fa0)
allocated by thread T0 (Web Content) here:
    #0 0x4b6338 in __interceptor_malloc _asan_rtl_
    #1 0x7f6b6610d31a in nsACString_internal::MutatePrep(unsigned int, char**, unsigned int*) xpcom/string/nsSubstring.cpp:217:22
    #2 0x7f6b6611b783 in nsACString_internal::SetCapacity(unsigned int, mozilla::fallible_t const&) xpcom/string/nsTSubstring.cpp:665:8
    #3 0x7f6b660fd3c1 in nsACString_internal::SetLength(unsigned int, mozilla::fallible_t const&) xpcom/string/nsTSubstring.cpp:703:8
    #4 0x7f6b66492c4e in nsStandardURL::BuildNormalizedSpec(char const*) netwerk/base/nsStandardURL.cpp:591:10
    #5 0x7f6b6649ae7d in nsStandardURL::SetSpec(nsACString_internal const&) netwerk/base/nsStandardURL.cpp:1191:14
    #6 0x7f6b664b07b9 in nsStandardURL::Init(unsigned int, int, nsACString_internal const&, char const*, nsIURI*) netwerk/base/nsStandardURL.cpp:2809:16
    #7 0x7f6b666df0bd in nsFileProtocolHandler::NewURI(nsACString_internal const&, char const*, nsIURI*, nsIURI**) netwerk/protocol/file/nsFileProtocolHandler.cpp:172:19
    #8 0x7f6b663ecd41 in nsIOService::NewURI(nsACString_internal const&, char const*, nsIURI*, nsIURI**) netwerk/base/nsIOService.cpp:609:12
    #9 0x7f6b664204a4 in NS_NewURI(nsIURI**, nsACString_internal const&, char const*, nsIURI*, nsIIOService*) netwerk/base/nsNetUtil.inl:102:14
    #10 0x7f6b6d31bbe5 in nsDefaultURIFixup::GetFixupURIInfo(nsACString_internal const&, unsigned int, nsIInputStream**, nsIURIFixupInfo**) docshell/base/nsDefaultURIFixup.cpp:307:10
    #11 0x7f6b6d365ea8 in nsDocShell::LoadURIWithOptions(char16_t const*, unsigned int, nsIURI*, unsigned int, nsIInputStream*, nsIInputStream*, nsIURI*) docshell/base/nsDocShell.cpp:4685:10
    #12 0x7f6b662745a1 in NS_InvokeByIndex xpcom/reflect/xptcall/md/unix/xptcinvoke_x86_64_unix.cpp:174:23
    #13 0x7f6b676a0508 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) js/xpconnect/src/XPCWrappedNative.cpp:2095:12
    #14 0x7f6b676a7263 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1145:12
    #15 0x7f6b6f705248 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/jscntxtinlines.h:235:15
    #16 0x7f6b6f74f354 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:3035:18
    #17 0x7f6b6f728e35 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:714:12
    #18 0x7f6b6f705952 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:791:15
    #19 0x7f6b6f69df6b in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:828:10
    #20 0x7f6b702ef244 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) js/src/jsapi.cpp:4594:12
    #21 0x7f6b684ba6bb in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, bool, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) dom/base/nsFrameMessageManager.cpp:1258:14
    #22 0x7f6b684b7ece in nsFrameMessageManager::ReceiveMessage(nsISupports*, nsIFrameLoader*, nsAString_internal const&, bool, mozilla::dom::StructuredCloneData const*, mozilla::jsipc::CpowHolder*, nsIPrincipal*, nsTArray<mozilla::OwningSerializedStructuredCloneBuffer>*) dom/base/nsFrameMessageManager.cpp:1075:10
    #23 0x7f6b6b7daeec in mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) dom/ipc/TabChild.cpp:2364:5
    #24 0x7f6b6b7db01c in non-virtual thunk to mozilla::dom::TabChild::RecvAsyncMessage(nsString const&, mozilla::dom::ClonedMessageData const&, nsTArray<mozilla::jsipc::CpowEntry>&&, IPC::Principal const&) dom/ipc/TabChild.cpp:2353:11
    #25 0x7f6b671df2e6 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PBrowserChild.cpp:2704:20
    #26 0x7f6b673378da in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) objdir-ff-asan/ipc/ipdl/PContentChild.cpp:5465:16
    #27 0x7f6b66b954ab in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp:1382:14
    #28 0x7f6b66b9299c in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp:1302:17
    #29 0x7f6b66b85037 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() ipc/glue/MessageChannel.cpp:1273:5
Shadow bytes around the buggy address:
  0x0c1e7fffe5a0: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fffe5b0: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c1e7fffe5c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e7fffe5d0: fd fd fd fd fd fd fa fa fa fa fa fa fa fa 00 00
  0x0c1e7fffe5e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c1e7fffe5f0: 00 00 00 00[fa]fa fa fa fa fa fa fa fd fd fd fd
  0x0c1e7fffe600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e7fffe610: fd fd fa fa fa fa fa fa fa fa fd fd fd fd fd fd
  0x0c1e7fffe620: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c1e7fffe630: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c1e7fffe640: 00 00 00 00 00 00 00 00 00 00 00 00 05 fa fa 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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==21063==ABORTING

Comment 1

[Daniel Veditz [:dveditz]]

sec-high might be overrating this, but looks like maybe a string with a missing null termination (or plain empty, given you're trying to create a service worker out of an empty file). Worst case if an attacker can rearrange memory so we don't crash maybe program data could be read into an error string the attacker can catch. If not then this would likely get a lot lower rating.

Comment 2

[Andrew McCreight [:mccr8]]

The particular failure that is generating this bogus string appears to be in ServiceWorkerContainer::Register, on this line:
  aRv.ThrowTypeError(MSG_INVALID_SCOPE, &aOptions.mScope.Value(), &spec);

I printed out the string in js_strlen and I got this: "QQQ string is file:///Users/amccreight/tests/1194562.html/,,,,,,,,,," and then the commas go on and on and on.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

This probably effects 42 as well.  We're shipping service worker registration on 42 to support push.

I think this is the code in SW that's crashing.  Its not clear to me why, though:

  https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerContainer.cpp#152

Comment 4

[Andrew McCreight [:mccr8]]

The problem is that ErrorResult::ThrowErrorWithMessage() uses varargs (so it is untyped) and it casts everything passed to it to nsAString, which is a wide character type, but the spec argument is nsAutoCString, which is a narrow type. If you paste this into where the ThrowTypeError call happens you can see that the first one is okay but the second produces an error:

     (void) static_cast<const nsAString*>(&aOptions.mScope.Value());
     (void) static_cast<const nsAString*>(&spec);

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Hmm, looks like we do that a few places in that file.

Comment 6

[Andrew McCreight [:mccr8]]

I'll look into using variadic templates for these DOM error functions so we get type safety.

Comment 7

[Andrew McCreight [:mccr8]]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> I'll look into using variadic templates for these DOM error functions so we
> get type safety.

I filed bug 1195977 for that.

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: fix

Convert to nsString before passing to ThrowTypeError

Comment 9

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8649490 [details] [diff] [review]
fix

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I am not sure, but based on dveditz's comment, not very easily.

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

Which older supported branches are affected by this flaw?
42 Dev Edition

If not all supported branches, which bug introduced the flaw?
We enable ServiceWorkers by default since 42, which could expose users to this flaw.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
This patch should apply without changes to 42.

How likely is this patch to cause regressions; how much testing does it need?
Unlikely to cause regressions, this just uses a string of the correct type.

Comment 10

[Andrew McCreight [:mccr8]]

Comment on attachment 8649490 [details] [diff] [review]
fix

Err, sorry, it looks like my analysis wasn't entirely right. I'll try to figure out what is actually going wrong...

Comment 11

[Andrew McCreight [:mccr8]]

Alright, I guess it is okay. I was just alarmed by the giant URL. I ran the test case before and after the patch in an ASan build and it fixes the buffer overflow.

Comment 12

[Andrew McCreight [:mccr8]]

Comment on attachment 8649490 [details] [diff] [review]
fix

See comment 9.

Comment 13

[Andrew McCreight [:mccr8]]

Also, please wait until I have gotten bug 1195977 far enough along that we can know if we have other instances of this in the tree, as we should fix everything at once.

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 8649490 [details] [diff] [review]
fix

sec-approval=dveditz (though technically unnecessary since I downgraded this to sec-moderate based on the analysis and patch).

[Triage Comment]
a=dveditz to land on aurora (42).

Comment 17

[Daniel Veditz [:dveditz]]

In theory this should be grabbing whole big gobs of unprotected memory into the error string, and if it did then maybe a sec-high rating is justified. but in a non-ASAN build I'm having trouble seeing it.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/02af2bd7f469
https://hg.mozilla.org/releases/mozilla-aurora/rev/f42fa2b6cc42

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/02af2bd7f469

Comment 20

[Ada [:adalucinet]]

Reproduced on Nightly asan build from 2015-08-15, under Ubuntu 12.04 64-bit: with the attached testcase, ‘AddressSanitizer: heap-buffer-overflow’ is displayed in terminal and the tab crashed.
No crash encountered with 42.0RC build 1 (Build ID: 20151026170526) and latest 43.0a2 asan build, across platforms [1].

[1] Ubuntu 12.04 64-bit, Mac OS X 10.11 and Windows 10 32-bit