Bug 45882 - WebCore..Frame..domWindow ReadAV@NULL (f8cd71f24fff11a7dbb6a39e738fe929)
Summary: WebCore..Frame..domWindow ReadAV@NULL (f8cd71f24fff11a7dbb6a39e738fe929)
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P1 Normal
Assignee: Berend-Jan Wever
URL:
Keywords:
: 43549 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-16 05:22 PDT by Berend-Jan Wever
Modified: 2023-12-30 02:14 PST (History)
3 users (show)

See Also:


Attachments
Repro (54 bytes, text/html)
2010-09-16 05:22 PDT, Berend-Jan Wever
no flags Details
Patch (1.67 KB, patch)
2011-01-25 01:41 PST, Berend-Jan Wever
skylined: review-
Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2011-03-09 07:42 PST, Berend-Jan Wever
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Berend-Jan Wever 2010-09-16 05:22:57 PDT
Created attachment 67783 [details]
Repro

<script>
  setTimeout(window.postMessage,1);
</script>

Result in Chromium latest:
id:             WebCore::Frame::domWindow ReadAV@NULL (f8cd71f24fff11a7dbb6a39e738fe929)
description:    Attempt to read from NULL pointer (+0x338) in WebCore::Frame::domWindow
stack:          WebCore::Frame::domWindow
                WebCore::V8DOMWindow::postMessageCallback
                v8::internal::HandleApiCallHelper<...>
                v8::internal::Builtin_HandleApiCall
                v8::internal::Invoke
                v8::internal::Execution::Call
                v8::Function::Call
                WebCore::V8Proxy::callFunction
                WebCore::ScheduledAction::execute
                WebCore::ScheduledAction::execute
                WebCore::DOMTimer::fired
                WebCore::ThreadTimers::sharedTimerFiredInternal
                MessageLoop::RunTask
                MessageLoop::DeferOrRunPendingTask
                MessageLoop::DoDelayedWork
                base::MessagePumpDefault::Run
                MessageLoop::RunInternal
                MessageLoop::Run
                RendererMain
                ChromeMain
                MainDllLoader::Launch
                wWinMain
                __tmainCRTStartup
                BaseThreadInitThunk
                RtlInitializeExceptionChain
                RtlInitializeExceptionChain
Comment 1 Alexey Proskuryakov 2010-09-16 11:37:32 PDT
Doesn't crash in Safari on Mac, and an exception is raised:

SYNTAX_ERR: DOM Exception 12: An invalid or illegal string was specified.
Comment 2 Berend-Jan Wever 2011-01-25 01:41:37 PST
Created attachment 80035 [details]
Patch

The problem is in v8 bindings; the code calls "V8Proxy::retrieveFrameForCallingContext" and assumes it always returns an object. It then calls a method on the returned value, which leads to the NULL pointer crash.

http://codesearch.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp&q=V8DOMWindow::postMessageCallback&exact_package=chromium&sa=N&cd=1&ct=rc&l=318
v8::Handle<v8::Value> V8DOMWindow::postMessageCallback(const v8::Arguments& args)
{
    INC_STATS("DOM.DOMWindow.postMessage()");
    DOMWindow* window = V8DOMWindow::toNative(args.Holder());

    DOMWindow* source = V8Proxy::retrieveFrameForCallingContext()->domWindow(); //  Call here, then crash w/ NULL ptr.

http://codesearch.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/third_party/WebKit/WebCore/bindings/v8/V8Proxy.cpp&q=V8Proxy::retrieveFrameForCallingContext&exact_package=chromium&sa=N&cd=1&ct=rc&l=565

Frame* V8Proxy::retrieveFrameForCallingContext()
{
    v8::Handle<v8::Context> context = v8::Context::GetCalling();
    if (context.IsEmpty())
        return 0;                                          // return NULL here
    return retrieveFrame(context);
}
Comment 3 Berend-Jan Wever 2011-02-17 04:04:17 PST
*** Bug 43549 has been marked as a duplicate of this bug. ***
Comment 4 Berend-Jan Wever 2011-03-02 04:14:38 PST
Comment on attachment 80035 [details]
Patch

I'll file a proper patch
Comment 5 Berend-Jan Wever 2011-03-09 07:42:39 PST
Created attachment 85175 [details]
Patch
Comment 6 Berend-Jan Wever 2011-03-11 05:49:24 PST
Chromium: http://code.google.com/p/chromium/issues/detail?id=55844

@abarth: It seems you introduced this in the fix for https://bugs.webkit.org/show_bug.cgi?id=26004 - does this fix make sense to you?
Comment 7 Adam Barth 2011-03-11 09:42:32 PST
Comment on attachment 85175 [details]
Patch

All patches need a ChangeLog.  Please see http://www.webkit.org/coding/contributing.html for instructions about preparing patches.

I'm not sure this change is correct.  The correct fix is to using the following idiom:

V8BindingState* state = V8BindingState::Only();
DOMWindow* activeWindow = state->activeWindow();
Comment 8 Anne van Kesteren 2023-12-30 02:14:23 PST
Chromium-specific.