856374 - Global-buffer-overflow in nsXMLContentSerializer::AppendToStringWrapped

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Abhishek Arya]

Attachment: Testcase

Load testcase, and click ctrl+s, enter to save document.

>==19307== ERROR: AddressSanitizer: global-buffer-overflow on address 0x7fd0d922511e at pc 0x7fd0be0b9e32 bp 0x7fff559271b0 sp 0x7fff559271a8
>READ of size 2 at 0x7fd0d922511e thread T0
>    #0 0x7fd0be0b9e31 in nsXMLContentSerializer::AppendToStringWrapped(nsAString_internal const&, nsAString_internal&) src/content/base/src/nsXMLContentSerializer.cpp:1692
>    #1 0x7fd0be0a2c89 in nsXHTMLContentSerializer::AppendText(nsIContent*, int, int, nsAString_internal&) src/content/base/src/nsXHTMLContentSerializer.cpp:150
>    #2 0x7fd0bdcc33e0 in nsDocumentEncoder::SerializeNodeStart(nsINode*, int, int, nsAString_internal&, nsINode*) src/content/base/src/nsDocumentEncoder.cpp:409
>    #3 0x7fd0bdcc6ad9 in nsDocumentEncoder::SerializeToStringRecursive(nsINode*, nsAString_internal&, bool) src/content/base/src/nsDocumentEncoder.cpp:493
>    #4 0x7fd0bdcc6f94 in nsDocumentEncoder::SerializeToStringRecursive(nsINode*, nsAString_internal&, bool) src/content/base/src/nsDocumentEncoder.cpp:502
>    #5 0x7fd0bdcc6f94 in nsDocumentEncoder::SerializeToStringRecursive(nsINode*, nsAString_internal&, bool) src/content/base/src/nsDocumentEncoder.cpp:502
>    #6 0x7fd0bdcc6f94 in nsDocumentEncoder::SerializeToStringRecursive(nsINode*, nsAString_internal&, bool) src/content/base/src/nsDocumentEncoder.cpp:502
>    #7 0x7fd0bdcc6f94 in nsDocumentEncoder::SerializeToStringRecursive(nsINode*, nsAString_internal&, bool) src/content/base/src/nsDocumentEncoder.cpp:502
>    #8 0x7fd0bdcd7f9c in nsDocumentEncoder::EncodeToString(nsAString_internal&) src/content/base/src/nsDocumentEncoder.cpp:1146
>    #9 0x7fd0bdcd9ee3 in nsDocumentEncoder::EncodeToStream(nsIOutputStream*) src/content/base/src/nsDocumentEncoder.cpp:1206
>    #10 0x7fd0c3f28fa6 in nsWebBrowserPersist::SaveDocumentWithFixup(nsIDOMDocument*, nsIDocumentEncoderNodeFixup*, nsIURI*, bool, nsACString_internal const&, nsCString const&, unsigned int) src/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:3773
>    #11 0x7fd0c3f0a443 in nsWebBrowserPersist::SaveDocuments() src/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:1770
>    #12 0x7fd0c3f0390a in nsWebBrowserPersist::SaveGatheredURIs(nsIURI*) src/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:562
>    #13 0x7fd0c3efe89e in nsWebBrowserPersist::SaveDocument(nsIDOMDocument*, nsISupports*, nsISupports*, char const*, unsigned int, unsigned int) src/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:455
>    #14 0x7fd0c3f04355 in non-virtual thunk to nsWebBrowserPersist::SaveDocument(nsIDOMDocument*, nsISupports*, nsISupports*, char const*, unsigned int, unsigned int) src/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:471
>    #15 0x7fd0c92e44cb in NS_InvokeByIndex_P src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
>    #16 0x7fd0c356809b in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) src/js/xpconnect/src/XPCWrappedNative.cpp:2953
>    #17 0x7fd0c35bbcab in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1457
>    #18 0x7fd0d0c5d15f in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jscntxtinlines.h:338
>    #19 0x7fd0d0c3641a in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2357
>    #20 0x7fd0d0be2acd in js::RunScript(JSContext*, js::StackFrame*) src/js/src/jsinterp.cpp:341
>    #21 0x7fd0d0c5d808 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:398
>    #22 0x7fd0d0687456 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) src/js/src/jsinterp.h:135
>    #23 0x7fd0d0c61926 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) src/js/src/jsinterp.cpp:431
>    #24 0x7fd0d059eb80 in JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) src/js/src/jsapi.cpp:5765
>    #25 0x7fd0c7527e16 in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JSObject*, nsDOMEvent&, mozilla::ErrorResult&) src/objdir-ff-asan-sym/dom/bindings/EventHandlerBinding.cpp:51
>    #26 0x7fd0c0a80039 in JS::Value mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, nsDOMEvent&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling) src/../../../dist/include/mozilla/dom/EventHandlerBinding.h:59
>    #27 0x7fd0c0a7ce83 in nsJSEventListener::HandleEvent(nsIDOMEvent*) src/dom/src/events/nsJSEventListener.cpp:249
>    #28 0x7fd0be817135 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsIDOMEventTarget*, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:927
>    #29 0x7fd0be818d94 in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:997
>    #30 0x7fd0bea0646b in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.h:277
>    #31 0x7fd0be9f62c7 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, bool, nsCxPusher*) src/content/events/src/nsEventDispatcher.cpp:200
>    #32 0x7fd0be9f3c8c in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, nsDispatchingCallback*, bool, nsCxPusher*) src/content/events/src/nsEventDispatcher.cpp:325
>    #33 0x7fd0be9fb8b3 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) src/content/events/src/nsEventDispatcher.cpp:631
>    #34 0x7fd0be9fde5d in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) src/content/events/src/nsEventDispatcher.cpp:691
>    #35 0x7fd0bde39e64 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) src/content/base/src/nsINode.cpp:1113
>    #36 0x7fd0bd98f225 in nsContentUtils::DispatchXULCommand(nsIContent*, bool, nsIDOMEvent*, nsIPresShell*, bool, bool, bool, bool) src/content/base/src/nsContentUtils.cpp:5909
>    #37 0x7fd0c20cc00b in nsXULElement::PreHandleEvent(nsEventChainPreVisitor&) src/content/xul/content/src/nsXULElement.cpp:1205
>    #38 0x7fd0be9f1263 in nsEventTargetChainItem::PreHandleEvent(nsEventChainPreVisitor&) src/content/events/src/nsEventDispatcher.cpp:259
>    #39 0x7fd0be9fa55e in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) src/content/events/src/nsEventDispatcher.cpp:557
>    #40 0x7fd0be9fde5d in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) src/content/events/src/nsEventDispatcher.cpp:691
>    #41 0x7fd0bc386d10 in PresShell::HandleDOMEventWithTarget(nsIContent*, nsIDOMEvent*, nsEventStatus*) src/layout/base/nsPresShell.cpp:7023
>    #42 0x7fd0bd98ef05 in nsContentUtils::DispatchXULCommand(nsIContent*, bool, nsIDOMEvent*, nsIPresShell*, bool, bool, bool, bool) src/content/base/src/nsContentUtils.cpp:5903
>    #43 0x7fd0bd63d660 in nsXULMenuCommandEvent::Run() src/layout/xul/base/src/nsXULPopupManager.cpp:2328
>    #44 0x7fd0c91c8187 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:627
>    #45 0x7fd0c8e75de2 in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:238
>    #46 0x7fd0c6083209 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82
>    #47 0x7fd0c948830b in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:216
>    #48 0x7fd0c948815e in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:209
>    #49 0x7fd0c9488049 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:183
>    #50 0x7fd0c54ba8be in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:163
>    #51 0x7fd0c418d800 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:288
>    #52 0x7fd0b9cde0ac in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3880
>    #53 0x7fd0b9ce3769 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3947
>    #54 0x7fd0b9ce6245 in XRE_main src/toolkit/xre/nsAppRunner.cpp:4152
>    #55 0x427f37 in do_main(int, char**, nsIFile*) src/browser/app/nsBrowserApp.cpp:232
>    #56 0x4250f1 in main src/browser/app/nsBrowserApp.cpp:533
>    #57 0x7fd0da2d976c in
>    #58 0x424864 in
>0x7fd0d922511e is located 2 bytes to the left of global variable '_ZL9gNullChar (src/xpcom/string/src/nsSubstring.cpp)' (0x7fd0d9225120) of size 2
>  '_ZL9gNullChar (src/xpcom/string/src/nsSubstring.cpp)' is ascii string ''
>0x7fd0d922511e is located 54 bytes to the right of global variable '_ZGVZ11NullCStringvE5sNull (src/xpcom/string/src/nsReadableUtils.cpp)' (0x7fd0d92250e0) of size 8
>  '_ZGVZ11NullCStringvE5sNull (src/xpcom/string/src/nsReadableUtils.cpp)' is ascii string ''
>Shadow bytes around the buggy address:
>  0x0ffa9b23c9d0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23c9e0: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23c9f0: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23ca00: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23ca10: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>=>0x0ffa9b23ca20: f9 f9 f9[f9]02 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23ca30: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
>  0x0ffa9b23ca40: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
>  0x0ffa9b23ca50: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
>  0x0ffa9b23ca60: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
>  0x0ffa9b23ca70: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
>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 righ 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
>  ASan internal:         fe
>==19307== ABORTING
>
>

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Henri, want to look at this, or should I?

Comment 2

[Henri Sivonen (:hsivonen)]

This looks rather harmless. It's a read before a buffer, the value read is only compared with constants and the decision taken based on the comparison is not security-sensitive.

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Check if the output string is empty

Comment 4

[Andrew McCreight [:mccr8]]

Marking sec-low based on comment 2.  It could maybe even be unhidden.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/87ddae234edc