Bug 218602 – WebGL2: Null pointer dereference in std::string implementation in gl::Shader::getTransformFeedbackVaryingMappedName

• Component: ANGLE
• Status: RESOLVED FIXED

[Ryosuke Niwa]

e.g.

==66584==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x0002adca6ac3 bp 0x7ffee71791d0 sp 0x7ffee71791d0 T0)
==66584==The signal is caused by a READ memory access.
==66584==Hint: address points to the zero page.
==66584==WARNING: invalid path to external symbolizer!
==66584==WARNING: Failed to use and restart external symbolizer!
    #0 0x2adca6ac3 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__is_long() const+0x23 (WebCore.framework/Versions/A/WebCore:x86_64+0x238ac3)
    #1 0x2adca6a7d in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__get_pointer() const+0xd (WebCore.framework/Versions/A/WebCore:x86_64+0x238a7d)
    #2 0x2adca6a38 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::data() const+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x238a38)
    #3 0x2add10827 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::append(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)+0x17 (WebCore.framework/Versions/A/WebCore:x86_64+0x2a2827)
    #4 0x2add107a3 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > std::__1::operator+<char, std::__1::char_traits<char>, std::__1::allocator<char> >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)+0x13 (WebCore.framework/Versions/A/WebCore:x86_64+0x2a27a3)
    #5 0x2b4027bcf in gl::Shader::getTransformFeedbackVaryingMappedName(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)+0x43f (WebCore.framework/Versions/A/WebCore:x86_64+0x65b9bcf)
    #6 0x2b3f0784a in rx::ProgramGL::link(gl::Context const*, gl::ProgramLinkedResources const&, gl::InfoLog&)+0x3da (WebCore.framework/Versions/A/WebCore:x86_64+0x649984a)
    #7 0x2b3eacf8d in gl::Program::linkImpl(gl::Context const*)+0xd0d (WebCore.framework/Versions/A/WebCore:x86_64+0x643ef8d)
    #8 0x2b3eac221 in gl::Program::link(gl::Context const*)+0x11 (WebCore.framework/Versions/A/WebCore:x86_64+0x643e221)
    #9 0x2b3ac0e3c in gl::Context::linkProgram(gl::ShaderProgramID)+0x1c (WebCore.framework/Versions/A/WebCore:x86_64+0x6052e3c)
    #10 0x2b3c3307f in gl::LinkProgram(unsigned int)+0x15f (WebCore.framework/Versions/A/WebCore:x86_64+0x61c507f)
    #11 0x2adbff713 in WebCore::GraphicsContextGLOpenGL::linkProgram(unsigned int)+0x13 (WebCore.framework/Versions/A/WebCore:x86_64+0x191713)
    #12 0x2b196c67c in WebCore::WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations(WebCore::WebGLProgram*)+0x21c (WebCore.framework/Versions/A/WebCore:x86_64+0x3efe67c)
    #13 0x2b196c43d in WebCore::WebGLRenderingContextBase::linkProgram(WebCore::WebGLProgram&)+0xd (WebCore.framework/Versions/A/WebCore:x86_64+0x3efe43d)
    #14 0x2af62974d in WebCore::jsWebGL2RenderingContextPrototypeFunctionLinkProgramBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWebGL2RenderingContext*)+0x22d (WebCore.framework/Versions/A/WebCore:x86_64+0x1bbb74d)
    #15 0x2af4ce61b in long long WebCore::IDLOperation<WebCore::JSWebGL2RenderingContext>::call<&(WebCore::jsWebGL2RenderingContextPrototypeFunctionLinkProgramBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWebGL2RenderingContext*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)+0xfb (WebCore.framework/Versions/A/WebCore:x86_64+0x1a6061b)
    #16 0x2af4ce518 in WebCore::jsWebGL2RenderingContextPrototypeFunctionLinkProgram(JSC::JSGlobalObject*, JSC::CallFrame*)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x1a60518)
    #17 0x3b6a84a01177  (<unknown module>)
    #18 0x2cc47fc58 in llint_entry+0x1beba (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xbc5c58)
    #19 0x2cc47fc58 in llint_entry+0x1beba (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xbc5c58)
    #20 0x2cc47fc58 in llint_entry+0x1beba (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xbc5c58)
    #21 0x2cc463ba8 in vmEntryToJavaScript+0xd7 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xba9ba8)
    #22 0x2cdbcc611 in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x611 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2312611)
    #23 0x2ce26d264 in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)+0x64 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x29b3264)
    #24 0x2ce26d35f in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)+0xdf (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x29b335f)
    #25 0x2ce26d71b in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)+0x10b (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x29b371b)
    #26 0x2b08f58e8 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)+0xe8 (WebCore.framework/Versions/A/WebCore:x86_64+0x2e878e8)
    #27 0x2b0921f1b in WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)+0xa7b (WebCore.framework/Versions/A/WebCore:x86_64+0x2eb3f1b)
    #28 0x2b119a8f2 in WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener>, WTF::DefaultRefDerefTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase)+0x522 (WebCore.framework/Versions/A/WebCore:x86_64+0x372c8f2)
    #29 0x2b1195672 in WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)+0x1b2 (WebCore.framework/Versions/A/WebCore:x86_64+0x3727672)
    #30 0x2b2135868 in WebCore::DOMWindow::dispatchEvent(WebCore::Event&, WebCore::EventTarget*)+0x2d8 (WebCore.framework/Versions/A/WebCore:x86_64+0x46c7868)
    #31 0x2b2148c57 in WebCore::DOMWindow::dispatchLoadEvent()+0x227 (WebCore.framework/Versions/A/WebCore:x86_64+0x46dac57)
    #32 0x2b104f655 in WebCore::Document::dispatchWindowLoadEvent()+0x55 (WebCore.framework/Versions/A/WebCore:x86_64+0x35e1655)
    #33 0x2b104f073 in WebCore::Document::implicitClose()+0x2f3 (WebCore.framework/Versions/A/WebCore:x86_64+0x35e1073)
    #34 0x2b1f47ba8 in WebCore::FrameLoader::checkCallImplicitClose()+0xd8 (WebCore.framework/Versions/A/WebCore:x86_64+0x44d9ba8)
    #35 0x2b1f47052 in WebCore::FrameLoader::checkCompleted()+0x2b2 (WebCore.framework/Versions/A/WebCore:x86_64+0x44d9052)
    #36 0x2b1f434e4 in WebCore::FrameLoader::finishedParsing()+0x1c4 (WebCore.framework/Versions/A/WebCore:x86_64+0x44d54e4)
    #37 0x2b106e803 in WebCore::Document::finishedParsing()+0x263 (WebCore.framework/Versions/A/WebCore:x86_64+0x3600803)
    #38 0x2b19b17e4 in WebCore::HTMLConstructionSite::finishedParsing()+0x24 (WebCore.framework/Versions/A/WebCore:x86_64+0x3f437e4)
    #39 0x2b1a15b0d in WebCore::HTMLTreeBuilder::finished()+0x1d (WebCore.framework/Versions/A/WebCore:x86_64+0x3fa7b0d)
    #40 0x2b19ba617 in WebCore::HTMLDocumentParser::end()+0x17 (WebCore.framework/Versions/A/WebCore:x86_64+0x3f4c617)
    #41 0x2b19b7df8 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd()+0x38 (WebCore.framework/Versions/A/WebCore:x86_64+0x3f49df8)
    #42 0x2b19b7cca in WebCore::HTMLDocumentParser::prepareToStopParsing()+0x10a (WebCore.framework/Versions/A/WebCore:x86_64+0x3f49cca)
    #43 0x2b19ba65f in WebCore::HTMLDocumentParser::attemptToEnd()+0x3f (WebCore.framework/Versions/A/WebCore:x86_64+0x3f4c65f)
    #44 0x2b19ba739 in WebCore::HTMLDocumentParser::finish()+0x29 (WebCore.framework/Versions/A/WebCore:x86_64+0x3f4c739)
    #45 0x2b1ecfa68 in WebCore::DocumentWriter::end()+0x1a8 (WebCore.framework/Versions/A/WebCore:x86_64+0x4461a68)
    #46 0x2b1ece52c in WebCore::DocumentLoader::finishedLoading()+0x2dc (WebCore.framework/Versions/A/WebCore:x86_64+0x446052c)
    #47 0x2b1ecde93 in WebCore::DocumentLoader::notifyFinished(WebCore::CachedResource&, WebCore::NetworkLoadMetrics const&)+0x2d3 (WebCore.framework/Versions/A/WebCore:x86_64+0x445fe93)
    #48 0x2b20951ff in WebCore::CachedResource::checkNotify(WebCore::NetworkLoadMetrics const&)+0x17f (WebCore.framework/Versions/A/WebCore:x86_64+0x46271ff)
    #49 0x2b208f82e in WebCore::CachedResource::finishLoading(WebCore::SharedBuffer*, WebCore::NetworkLoadMetrics const&)+0x4e (WebCore.framework/Versions/A/WebCore:x86_64+0x462182e)
    #50 0x2b20910e8 in WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*, WebCore::NetworkLoadMetrics const&)+0x258 (WebCore.framework/Versions/A/WebCore:x86_64+0x46230e8)
    #51 0x2b2004d82 in WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&)+0x732 (WebCore.framework/Versions/A/WebCore:x86_64+0x4596d82)
    #52 0x2a1aff3b6 in WebKit::WebResourceLoader::didFinishResourceLoad(WebCore::NetworkLoadMetrics const&)+0x286 (WebKit.framework/Versions/A/WebKit:x86_64+0x1afd3b6)
    #53 0x2a221e251 in void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, 0ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>&&, std::__1::integer_sequence<unsigned long, 0ul>)+0x61 (WebKit.framework/Versions/A/WebKit:x86_64+0x221c251)
    #54 0x2a221e1d8 in void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebCore::NetworkLoadMetrics>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))+0x28 (WebKit.framework/Versions/A/WebKit:x86_64+0x221c1d8)
    #55 0x2a221bc46 in void IPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))+0x146 (WebKit.framework/Versions/A/WebKit:x86_64+0x2219c46)
    #56 0x2a221b253 in WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&)+0x1a3 (WebKit.framework/Versions/A/WebKit:x86_64+0x2219253)
    #57 0x2a1ac01aa in WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)+0xfa (WebKit.framework/Versions/A/WebKit:x86_64+0x1abe1aa)
    #58 0x2a009f95e in IPC::Connection::dispatchMessage(IPC::Decoder&)+0x1ce (WebKit.framework/Versions/A/WebKit:x86_64+0x9d95e)
    #59 0x2a00a05f7 in IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)+0x167 (WebKit.framework/Versions/A/WebKit:x86_64+0x9e5f7)
    #60 0x2a00a1116 in IPC::Connection::dispatchOneIncomingMessage()+0x196 (WebKit.framework/Versions/A/WebKit:x86_64+0x9f116)
    #61 0x2a00be1d5 in IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7::operator()()+0x35 (WebKit.framework/Versions/A/WebKit:x86_64+0xbc1d5)
    #62 0x2a00be13c in WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_7, void>::call()+0xc (WebKit.framework/Versions/A/WebKit:x86_64+0xbc13c)
    #63 0x2cb8f247e in WTF::Function<void ()>::operator()() const+0x3e (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x3847e)
    #64 0x2cb989f38 in WTF::RunLoop::performWork()+0x228 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xcff38)
    #65 0x2cb98d175 in WTF::RunLoop::performWork(void*)+0xb5 (JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xd3175)
    #66 0x7fff205089fb in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__+0x10 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64h+0x819fb)
    #67 0x7fff20508963 in __CFRunLoopDoSource0+0xb3 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64h+0x81963)
    #68 0x7fff205086de in __CFRunLoopDoSources0+0xf7 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64h+0x816de)
    #69 0x7fff20507110 in __CFRunLoopRun+0x379 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64h+0x80110)
    #70 0x7fff205066bd in CFRunLoopRunSpecific+0x232 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64h+0x7f6bd)
    #71 0x7fff21290fa0 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:]+0xd3 (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x5ffa0)
    #72 0x7fff2131f383 in -[NSRunLoop(NSRunLoop) run]+0x4b (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0xee383)
    #73 0x7fff2015f3dc in _xpc_objc_main+0x338 (/usr/lib/system/libxpc.dylib:x86_64+0x153dc)
    #74 0x7fff2015ee64 in xpc_main+0x1b4 (/usr/lib/system/libxpc.dylib:x86_64+0x14e64)
    #75 0x2a0a956cf in WebKit::XPCServiceMain(int, char const**)+0x59f (WebKit.framework/Versions/A/WebKit:x86_64+0xa936cf)
    #76 0x2a22d57a8 in WKXPCServiceMain+0x8 (WebKit.framework/Versions/A/WebKit:x86_64+0x22d37a8)
    #77 0x108a84e28 in main+0x8 (com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development:x86_64+0x100003e28)
    #78 0x7fff2042b590 in start+0x0 (/usr/lib/system/libdyld.dylib:x86_64+0x16590)

<rdar://problem/69610382>

[Ryosuke Niwa]

Created attachment 413258 [details]
Test case

[Rob Buis]

Created attachment 414349 [details]
Patch

[Rob Buis]

For the test case the varying "matrix" is queried for field "vector" (the only varyings here that I can see are "vector" and "matrix"). "matrix" is deemed a struct but the field vector is not found on it, resulting in a null pointer. Then we either hit the assert or the actual pointer dereference.

[Rob Buis]

I wonder if this would need to be fixed in ANGLE repo first?

[Ryosuke Niwa]

Comment on attachment 414349 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414349&action=review

> Source/ThirdParty/ANGLE/src/libANGLE/Shader.cpp:660
> +                if (!field)
> +                    continue;
> +                ASSERT(!field->isStruct() && !field->isArray());

Is this just a nullptr crash or is there any security implication here?

[Rob Buis]

(In reply to Ryosuke Niwa from comment #5)
> Comment on attachment 414349 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414349&action=review
> 
> > Source/ThirdParty/ANGLE/src/libANGLE/Shader.cpp:660
> > +                if (!field)
> > +                    continue;
> > +                ASSERT(!field->isStruct() && !field->isArray());
> 
> Is this just a nullptr crash or is there any security implication here?

I debugged this, yes it is a nullptr crash and thus hits the first condition in the ASSERT.

[Ryosuke Niwa]

Comment on attachment 414349 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414349&action=review

>>> Source/ThirdParty/ANGLE/src/libANGLE/Shader.cpp:660
>>> +                ASSERT(!field->isStruct() && !field->isArray());
>> 
>> Is this just a nullptr crash or is there any security implication here?
> 
> I debugged this, yes it is a nullptr crash and thus hits the first condition in the ASSERT.

In release builds? Sometimes nullptr crash in a debug build results in a security bug in release builds.

[Rob Buis]

Created attachment 415225 [details]
Patch

[Rob Buis]

(In reply to Ryosuke Niwa from comment #7)
> Comment on attachment 414349 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=414349&action=review
> 
> >>> Source/ThirdParty/ANGLE/src/libANGLE/Shader.cpp:660
> >>> +                ASSERT(!field->isStruct() && !field->isArray());
> >> 
> >> Is this just a nullptr crash or is there any security implication here?
> > 
> > I debugged this, yes it is a nullptr crash and thus hits the first condition in the ASSERT.
> 
> In release builds? Sometimes nullptr crash in a debug build results in a
> security bug in release builds.

Yes it is a crash in release and debug builds, independent of the ASSERT the next line will cause a crash for sure:
return varying.mappedName + "." + field->mappedName;

I made a patch that fixes the crash, I wonder if it would need to go to ANGLE first?

[Dean Jackson]

The ANGLE folks will see this and pull it in to their upstream.

[Ryosuke Niwa]

There is no security implication here, right?

[Ryosuke Niwa]

Comment on attachment 415225 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415225&action=review

> Source/ThirdParty/ANGLE/ChangeLog:3
> +        Skip varying if field is not found

Can we match the bug title here?

> LayoutTests/ChangeLog:3
> +        Skip varying if field is not found

Ditto.

[Alex Christensen]

Definitely no UAF here.

[Rob Buis]

Created attachment 415392 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[EWS]

Committed r270426: <https://trac.webkit.org/changeset/270426>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415392 [details].