Bug 57140 – Crash from null pointer dereference below WebCore::StorageAreaImpl::setItem()

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Created attachment 87009 [details]
Reduced test case (WILL CRASH!)

Null pointer dereference below WebCore::StorageAreaImpl::setItem() causes a crash with the attached test case.

Crashes WebKit nightly build r82003 with Safari 5.0.4 on SnowLeopard 10.6.7:

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00000001017f59d8 WebCore::privateBrowsingEnabled(WebCore::Frame*) + 8
1   com.apple.WebCore             	0x00000001017f6462 WebCore::StorageAreaImpl::setItem(WTF::String const&, WTF::String const&, int&, WebCore::Frame*) + 66
2   com.apple.WebCore             	0x00000001017f57d2 WebCore::Storage::setItem(WTF::String const&, WTF::String const&, int&) + 50
3   com.apple.WebCore             	0x00000001013db6bf WebCore::JSStorage::putDelegate(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&) + 1695
4   com.apple.WebCore             	0x00000001013d8970 WebCore::JSStorage::put(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&) + 48
5   com.apple.JavaScriptCore      	0x000000010081f87c cti_op_put_by_id + 108
6   ???                           	0x0000424d1061faf7 0 + 72898754771703
7   com.apple.JavaScriptCore      	0x00000001007d942f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 847
8   com.apple.JavaScriptCore      	0x000000010079aa3d JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 45
9   com.apple.WebCore             	0x000000010129da41 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) + 2145
10  com.apple.WebCore             	0x0000000100f6cf80 WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) + 240
11  com.apple.WebCore             	0x0000000100f6d842 WebCore::EventTarget::fireEventListeners(WebCore::Event*) + 146
12  com.apple.WebCore             	0x0000000100f1d086 WebCore::DOMWindow::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WTF::PassRefPtr<WebCore::EventTarget>) + 278
13  com.apple.WebCore             	0x0000000100f1d19a WebCore::DOMWindow::dispatchTimedEvent(WTF::PassRefPtr<WebCore::Event>, WebCore::Document*, double*, double*) + 106
14  com.apple.WebCore             	0x0000000100f1f2bb WebCore::DOMWindow::dispatchLoadEvent() + 955
15  com.apple.WebCore             	0x0000000100e4efa4 WebCore::Document::implicitClose() + 260
16  com.apple.WebCore             	0x0000000100fb3972 WebCore::FrameLoader::checkCompleted() + 194
17  com.apple.WebCore             	0x0000000100fb40b3 WebCore::FrameLoader::finishedParsing() + 131
18  com.apple.WebCore             	0x0000000100e5b0cf WebCore::Document::finishedParsing() + 239
19  com.apple.WebCore             	0x00000001010655c7 WebCore::HTMLDocumentParser::prepareToStopParsing() + 87
20  com.apple.WebCore             	0x00000001010634dc WebCore::HTMLDocumentParser::finish() + 44
21  com.apple.WebCore             	0x0000000100e74618 WebCore::DocumentWriter::endIfNotLoadingMainResource() + 88
22  com.apple.WebCore             	0x0000000100fb377a WebCore::FrameLoader::finishedLoading() + 90
23  com.apple.WebCore             	0x000000010159ba93 WebCore::MainResourceLoader::didFinishLoading(double) + 147
24  com.apple.Foundation          	0x00007fff8365f608 _NSURLConnectionDidFinishLoading + 113
25  com.apple.CFNetwork           	0x00007fff80e631a0 URLConnectionClient::_clientDidFinishLoading(URLConnectionClient::ClientConnectionEventQueue*) + 174
26  com.apple.CFNetwork           	0x00007fff80ec89ae URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<XClientEvent, XClientEventParams>*, long) + 254
27  com.apple.CFNetwork           	0x00007fff80ec8c1a URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<XClientEvent, XClientEventParams>*, long) + 874
28  com.apple.CFNetwork           	0x00007fff80e4f825 URLConnectionClient::processEvents() + 121
29  com.apple.CFNetwork           	0x00007fff80e4f600 MultiplexerSource::perform() + 160
30  com.apple.CoreFoundation      	0x00007fff888d8401 __CFRunLoopDoSources0 + 1361
31  com.apple.CoreFoundation      	0x00007fff888d65f9 __CFRunLoopRun + 873
32  com.apple.CoreFoundation      	0x00007fff888d5dbf CFRunLoopRunSpecific + 575
33  com.apple.HIToolbox           	0x00007fff86ea37ee RunCurrentEventLoopInMode + 333
34  com.apple.HIToolbox           	0x00007fff86ea35f3 ReceiveNextEventCommon + 310
35  com.apple.HIToolbox           	0x00007fff86ea34ac BlockUntilNextEventMatchingListInMode + 59
36  com.apple.AppKit              	0x00007fff83c00e64 _DPSNextEvent + 718
37  com.apple.AppKit              	0x00007fff83c007a9 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 155
38  com.apple.Safari              	0x000000010001605a 0x100000000 + 90202
39  com.apple.AppKit              	0x00007fff83bc648b -[NSApplication run] + 395
40  com.apple.AppKit              	0x00007fff83bbf1a8 NSApplicationMain + 364
41  com.apple.Safari              	0x0000000100009f7c 0x100000000 + 40828

[David Kilzer (:ddkilzer)]

<rdar://problem/9191214>

[Jeremy Orlow]

FWIW, this doesn't crash even the renderer in Chrome.  I'm a little surprised there's a difference though.

[Julien Chaffraix]

Created attachment 94469 [details]
Proposed fix: NULL-check for page like the rest of the code

[Darin Adler]

Comment on attachment 94469 [details]
Proposed fix: NULL-check for page like the rest of the code

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

> Source/WebCore/storage/StorageAreaImpl.cpp:106
> -    return frame->page()->settings()->privateBrowsingEnabled();
> +    return frame->page() ? frame->page()->settings()->privateBrowsingEnabled() : false;

I like writing these with && instead of ? : like this:

    return frame->page() && frame->page()->settings()->privateBrowsingEnabled();

[WebKit Review Bot]

Comment on attachment 94469 [details]
Proposed fix: NULL-check for page like the rest of the code

Attachment 94469 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8726482

New failing tests:
fast/storage/storage-detached-iframe.html

[WebKit Review Bot]

Created attachment 94484 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Julien Chaffraix]

The failure is a crash on Chromium-Linux. However the stack-trace is not helpful so I will proceed to land the patch updating test_expectations.txt as I think it is unrelated. Filed bug 61326 to cover the crash.

[Julien Chaffraix]

Committed r87114: <http://trac.webkit.org/changeset/87114>

[Ademar Reis]

Revision r87114 cherry-picked into qtwebkit-2.2 with commit 6d4125a <http://gitorious.org/webkit/qtwebkit/commit/6d4125a>