Bug 107556 – [chromium] Use after free in plugins/geturlnotify-during-document-teardown.html

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

The layout test invokes didAddMessageToConsole on a deleted WebViewHost. If you apply the attched patch and run the test, I get the following output:

~0x7f15b70b9a20
0x7f15b70b9a20:didAddMessageToConsole
CONSOLE MESSAGE: PLUGIN: NPP_Destroy
Content-Type: text/plain
This tests that performing a load during document teardown does not cause a crash. Bug #38797
#EOF
#EOF
~0x7f15b8f05e20
LEAK: 17 WebCoreNode

Created attachment 183985 [details]
Patch

Here's the backtrace to the WebViewHost destructor: 

#0  WebViewHost::~WebViewHost (this=0x7fffea69e820, vtt=0x5e62ab8 <VTT for WebTestRunner::WebTestProxy<WebViewHost, TestShell*>+8>)
    at ../../Tools/DumpRenderTree/chromium/WebViewHost.cpp:1137
#1  0x000000000044ac88 in WebTestRunner::WebTestProxy<WebViewHost, TestShell*>::~WebTestProxy (this=0x7fffea69e820, 
    vtt=0x5e62ab0 <VTT for WebTestRunner::WebTestProxy<WebViewHost, TestShell*>>)
    at ../../Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:159
#2  0x0000000000449120 in WebTestRunner::WebTestProxy<WebViewHost, TestShell*>::~WebTestProxy (this=0x7fffea69e820)
    at ../../Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:159
#3  0x0000000000449159 in WebTestRunner::WebTestProxy<WebViewHost, TestShell*>::~WebTestProxy (this=0x7fffea69e820)
    at ../../Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:159
#4  0x000000000044377d in TestShell::closeWindow (this=0x7fffffffdc08, window=0x7fffea69e820)
    at ../../Tools/DumpRenderTree/chromium/TestShell.cpp:798
#5  0x000000000045258b in WebViewHost::closeWidget (this=0x7fffea69e820) at ../../Tools/DumpRenderTree/chromium/WebViewHost.cpp:562
#6  0x000000000045c58b in WebViewHost::HostMethodTask::runIfValid (this=0x7fffe99fef80)
    at ../../Tools/DumpRenderTree/chromium/WebViewHost.h:297
#7  0x000000000045c4cf in WebTestRunner::WebMethodTask<WebViewHost>::run (this=0x7fffe99fef80)
    at ../../Tools/DumpRenderTree/chromium/TestRunner/public/WebTask.h:84
#8  0x000000000043f23d in (anonymous namespace)::TaskWrapper::Run (this=0x7fffe9e1f3e0) at ../../Tools/DumpRenderTree/chromium/Task.cpp:62
#9  0x0000000000461192 in base::internal::RunnableAdapter<void (webkit_support::TaskAdaptor::*)()>::Run (this=0x7fffffffc7e0, 
    object=0x7fffe9e1f3e0) at ../../Source/WebKit/chromium/base/bind_internal.h:134
#10 0x00000000004610fc in base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void (webkit_support::TaskAdaptor::*)()>, void (webkit_support::TaskAdaptor*)>::MakeItSo(base::internal::RunnableAdapter<void (webkit_support::TaskAdaptor::*)()>, webkit_support::TaskAdaptor*) (runnable=..., a1=0x7fffe9e1f3e0) at ../../Source/WebKit/chromium/base/bind_internal.h:871
#11 0x00000000004610a5 in base::internal::Invoker<1, base::internal::BindState<base::internal::RunnableAdapter<void (webkit_support::TaskAdaptor::*)()>, void (webkit_support::TaskAdaptor*), void (base::internal::OwnedWrapper<webkit_support::TaskAdaptor>)>, void (webkit_support::TaskAdaptor*)>::Run(base::internal::BindStateBase*) (base=0x7fffe99fe020) at ../../Source/WebKit/chromium/base/bind_internal.h:1173
#12 0x00000000005ee64e in base::Callback<void ()>::Run() const (this=0x7fffffffcb98) at ../../Source/WebKit/chromium/base/callback.h:396
#13 0x0000000001b6e6e0 in MessageLoop::RunTask (this=0x7ffff7ea5b20, pending_task=...)
    at ../../Source/WebKit/chromium/base/message_loop.cc:473
#14 0x0000000001b6ea9b in MessageLoop::DeferOrRunPendingTask (this=0x7ffff7ea5b20, pending_task=...)
    at ../../Source/WebKit/chromium/base/message_loop.cc:485
#15 0x0000000001b6ec45 in MessageLoop::DoWork (this=0x7ffff7ea5b20) at ../../Source/WebKit/chromium/base/message_loop.cc:668
#16 0x0000000001be90b2 in base::MessagePumpGlib::RunWithDispatcher (this=0x7ffff7ea6f20, delegate=0x7ffff7ea5b20, dispatcher=0x0)
    at ../../Source/WebKit/chromium/base/message_pump_glib.cc:203
#17 0x0000000001be9689 in base::MessagePumpGlib::Run (this=0x7ffff7ea6f20, delegate=0x7ffff7ea5b20)
    at ../../Source/WebKit/chromium/base/message_pump_glib.cc:296
#18 0x0000000001b6e146 in MessageLoop::RunInternal (this=0x7ffff7ea5b20) at ../../Source/WebKit/chromium/base/message_loop.cc:430
#19 0x0000000001b6dff5 in MessageLoop::RunHandler (this=0x7ffff7ea5b20) at ../../Source/WebKit/chromium/base/message_loop.cc:403
#20 0x0000000001ba15d2 in base::RunLoop::Run (this=0x7fffffffd038) at ../../Source/WebKit/chromium/base/run_loop.cc:45
#21 0x0000000001b6d891 in MessageLoop::Run (this=0x7ffff7ea5b20) at ../../Source/WebKit/chromium/base/message_loop.cc:310
#22 0x000000000045e821 in webkit_support::RunMessageLoop () at ../../Source/WebKit/chromium/webkit/support/webkit_support.cc:571
#23 0x000000000044f278 in TestShell::waitTestFinished (this=0x7fffffffdc08) at ../../Tools/DumpRenderTree/chromium/TestShellPosix.cpp:66
#24 0x0000000000443c94 in TestShell::runFileTest (this=0x7fffffffdc08, params=..., shouldDumpPixels=false)
    at ../../Tools/DumpRenderTree/chromium/TestShell.cpp:304
#25 0x0000000000420c0d in runTest (shell=..., params=..., inputLine=..., forceDumpPixels=false)
    at ../../Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:115
#26 0x000000000042084e in main (argc=3, argv=0x7fffffffdff8) at ../../Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:275

and here's the backtrace to didAddMessageToConsole:

#0  WebViewHost::didAddMessageToConsole (this=0x7fffea69e820, message=..., sourceName=..., sourceLine=0)
    at ../../Tools/DumpRenderTree/chromium/WebViewHost.cpp:226
#1  0x00000000005106ba in WebKit::ChromeClientImpl::addMessageToConsole (this=0x7fffe9ba30a0, source=WebCore::ConsoleAPIMessageSource, 
    level=WebCore::LogMessageLevel, message=..., lineNumber=0, sourceID=...) at ../../Source/WebKit/chromium/src/ChromeClientImpl.cpp:421
#2  0x0000000000514e32 in WebCore::ChromeClient::addMessageToConsole (this=0x7fffe9ba30a0, source=WebCore::ConsoleAPIMessageSource, 
    level=WebCore::LogMessageLevel, message=..., lineNumber=0, sourceID=...) at ../../Source/WebCore/page/ChromeClient.h:137
#3  0x0000000003044fe6 in WebCore::internalAddMessage (page=0x7fffea694a20, type=WebCore::LogMessageType, level=WebCore::LogMessageLevel, 
    state=0x7fffe9ac24d0, prpArguments=..., acceptNoArguments=false, printTrace=false) at ../../Source/WebCore/page/Console.cpp:210
#4  0x0000000003044cc7 in WebCore::Console::log (this=0x7fffe9ceb890, state=0x7fffe9ac24d0, arguments=...)
    at ../../Source/WebCore/page/Console.cpp:252
#5  0x0000000000896a7d in WebCore::ConsoleV8Internal::logCallback (args=...) at gen/webcore/bindings/V8Console.cpp:124
#6  0x00000000023b6685 in v8::internal::HandleApiCallHelper<false> (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1350
#7  0x00000000023b6263 in v8::internal::Builtin_Impl_HandleApiCall (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1368
#8  0x00000000023af58c in v8::internal::Builtin_HandleApiCall (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1367

I guess the bt to closeWidgetSoon is also interesting:

#0  WebViewHost::closeWidgetSoon (this=0x7fffea6b3820) at ../../Tools/DumpRenderTree/chromium/WebViewHost.cpp:568
#1  0x000000000051087e in WebKit::ChromeClientImpl::closeWindowSoon (this=0x7fffe9d298a0)
    at ../../Source/WebKit/chromium/src/ChromeClientImpl.cpp:451
#2  0x0000000003041e6d in WebCore::Chrome::closeWindowSoon (this=0x7fffe9d76db0) at ../../Source/WebCore/page/Chrome.cpp:301
#3  0x000000000306b275 in WebCore::DOMWindow::close (this=0x7fffea7bd3a0, context=0x7fffea6360c0)
    at ../../Source/WebCore/page/DOMWindow.cpp:989
#4  0x0000000000a1becc in WebCore::DOMWindowV8Internal::closeCallback (args=...) at gen/webcore/bindings/V8DOMWindow.cpp:2708
#5  0x00000000023b6685 in v8::internal::HandleApiCallHelper<false> (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1350
#6  0x00000000023b6263 in v8::internal::Builtin_Impl_HandleApiCall (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1368
#7  0x00000000023af58c in v8::internal::Builtin_HandleApiCall (args=..., isolate=0x7ffff7e62020)
    at ../../Source/WebKit/chromium/v8/src/builtins.cc:1367

There's also http://crbug.com/108833 that says this test crashes on Windows and consequently it's disabled on chromium-win

Adding Tony who filed the bug, and Darin who might know about WebWidget / ChromeClientImpl lifetime

Created attachment 184204 [details]
Patch

[Tony Chang]

Comment on attachment 184204 [details]
Patch

FWIW, this LGTM.

[Tony Chang]

To be clear, there's no security bug in Chromium, right?  This is just a use-after-free in DRT and test_shell.

Right, it only affects chromium DRT. Removing the security labels..

[WebKit Review Bot]

Comment on attachment 184204 [details]
Patch

Clearing flags on attachment: 184204

Committed r140561: <http://trac.webkit.org/changeset/140561>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Erik Arvidsson]

Reverted r140561 for reason:

Suspected to break Android which prevens WebKit roll

Committed r140703: <http://trac.webkit.org/changeset/140703>

[Erik Arvidsson]

This patch seems to expose a bug with Android WebView implementation.

[Tony Chang]

(In reply to comment #13)
> This patch seems to expose a bug with Android WebView implementation.

That doesn't make sense.  This is only code in DumpRenderTree.

[Erik Arvidsson]

I'm still getting the same Android test errors. I'm going to rollback the rollback.

[Erik Arvidsson]

Reverted r140703 for reason:

r140561 was not the reason for the Android breakage

Committed r140717: <http://trac.webkit.org/changeset/140717>