New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 0 users
Status: Fixed
Owner:
back Nov 29
Closed: Apr 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Heap-use-after-free in WebCore::V8AbstractEventListener::~V8AbstractEventListener
Project Member Reported by infe...@chromium.org, Apr 3 2012 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=33011032

Fuzzer: Inferno_layout_test_fuzzer

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x7f3c44a55360
Crash State:
  - crash stack -
  WebCore::V8AbstractEventListener::~V8AbstractEventListener
  WebCore::V8WorkerContextEventListener::~V8WorkerContextEventListener
  - free stack -
  v8::internal::GlobalHandles::~GlobalHandles
  v8::internal::GlobalHandles::Create
  
Regressed: https://cluster-fuzz.appspot.com/revisions?range=118266:118281

Minimized Testcase (1.73 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97WazXiKEnecNEq7CaBLCrc5oJXBi0STRx0w5XSsmB72QNvvr1Xte7uWSZ5Ae15AIl1vadu6BG3Pgo9c49bWQr1CfnC5t9vIgMob4oBRz4rueEz7lu21ufK0vSh6tXyyOdZtWohag7iDogQ8Z9qG9AQW3mIgA
 
Cc: adamk@chromium.org abarth@chromium.org
Cc: jsb...@chromium.org dgro...@chromium.org tony@chromium.org rolandsteiner@chromium.org
Labels: Feature-IndexedDB
Could this be coming from revert from https://trac.webkit.org/changeset/105378/. I cannot see anything else in that narrow regression range - https://trac.webkit.org/log/?verbose=on&stop_rev=105378&rev=105390&limit=1000

dgrogan@, jsbell@, any idea about this ?


Cc: -dgro...@chromium.org
Owner: dgro...@chromium.org
Status: Assigned
If that minimized test case is accurate it's not hitting the change in https://trac.webkit.org/changeset/105378/ at all; the change set only applies if the IDBObjectStore had a keyPath and autoIncrement was set, which are not the defaults and not set in that test case.

Ah, the revision range on the Chromium side is: r118266:r118281
... which includes:

http://src.chromium.org/viewvc/chrome?view=rev&revision=118267
Always enable indexeddb for workers.  Remove --enable-indexeddb-for-workers switch.

I believe that revision range was identified simply because that's when IDB+Workers would have been exposed to fuzzing; the actual issue would have been latent before then.
This looks similar to an Google-internal report that came across our radar this morning - sorry non-Googlers:

http://crash/reportdetail?reportid=8b362d0e3084ab98#crashing_thread

... and stack of the thread which crashed with EXCEPTION_ACCESS_VIOLATION_READ:

0x65b07a20	 [chrome.dll	 - v8.h:3956]	v8::Local<v8::Function>::New(v8::Handle<v8::Function>)
0x6619a79d	 [chrome.dll	 - v8abstracteventlistener.cpp:71]	WebCore::V8AbstractEventListener::~V8AbstractEventListener()
0x663cd68b	 [chrome.dll	 + 0x00b2d68b]	WebCore::V8WorkerContextEventListener::`scalar deleting destructor'(unsigned int)
0x661f102f	 [chrome.dll	 - vector.h:58]	WTF::VectorDestructor<1,WebCore::RegisteredEventListener>::destruct(WebCore::RegisteredEventListener *,WebCore::RegisteredEventListener *)
0x66349c46	 [chrome.dll	 + 0x00aa9c46]	WTF::Vector<WebCore::RegisteredEventListener,1>::`scalar deleting destructor'(unsigned int)
0x65b60c14	 [chrome.dll	 - hashtable.h:931]	WTF::HashTable<WTF::AtomicString,std::pair<WTF::AtomicString,WTF::OwnPtr<WTF::Vector<WebCore::RegisteredEventListener,1> > >,WTF::PairFirstExtractor<std::pair<WTF::AtomicString,WTF::OwnPtr<WTF::Vector<WebCore::RegisteredEventListener,1> > > >,WTF::AtomicStringHash,WTF::PairHashTraits<WebCore::EventListenerMap::EventListenerHashMapTraits,WTF::HashTraits<WTF::OwnPtr<WTF::Vector<WebCore::RegisteredEventListener,1> > > >,WebCore::EventListenerMap::EventListenerHashMapTraits>::deallocateTable(std::pair<WTF::AtomicString,WTF::OwnPtr<WTF::Vector<WebCore::RegisteredEventListener,1> > > *,int)
0x6636a55c	 [chrome.dll	 - eventtarget.cpp:78]	WebCore::EventTargetData::~EventTargetData()
0x6613ecfa	 [chrome.dll	 - idbrequest.cpp:77]	WebCore::IDBRequest::~IDBRequest()
0x6613ec99	 [chrome.dll	 + 0x0089ec99]	WebCore::IDBRequest::`scalar deleting destructor'(unsigned int)
0x66105ccf	 [chrome.dll	 - threadsaferefcounted.h:137]	WTF::ThreadSafeRefCounted<WebCore::IDBCursorBackendInterface>::deref()
0x66b18ef0	 [chrome.dll	 - webidbcallbacksimpl.cpp:56]	WebKit::WebIDBCallbacksImpl::~WebIDBCallbacksImpl()
0x66b18f75	 [chrome.dll	 + 0x01278f75]	WebKit::WebIDBCallbacksImpl::`scalar deleting destructor'(unsigned int)
0x658d8376	 [chrome.dll	 - id_map.h:184]	IDMap<fileapi::FileSystemCallbackDispatcher,1>::Releaser<1,0>::release_all(stdext::hash_map<int,fileapi::FileSystemCallbackDispatcher *,stdext::hash_compare<int,std::less<int> >,std::allocator<std::pair<int const ,fileapi::FileSystemCallbackDispatcher *> > > *)
0x658d8323	 [chrome.dll	 - id_map.h:52]	IDMap<WebKit::WebIDBCallbacks,1>::~IDMap<WebKit::WebIDBCallbacks,1>()
0x66b9f062	 [chrome.dll	 - indexed_db_dispatcher.cc:52]	IndexedDBDispatcher::~IndexedDBDispatcher()
0x66b9fe2e	 [chrome.dll	 + 0x012ffe2e]	IndexedDBDispatcher::`vector deleting destructor'(unsigned int)
0x658db215	 [chrome.dll	 - render_view_observer.cc:32]	webkit_database::DatabaseQuotaClient::OnQuotaManagerDestroyed()
0x668271c0	 [chrome.dll	 - worker_task_runner.cc:92]	webkit_glue::WorkerTaskRunner::OnWorkerRunLoopStopped(WebKit::WebWorkerRunLoop const &)
0x66b04ad3	 [chrome.dll	 - platformsupport.cpp:1130]	WebCore::PlatformSupport::didStopWorkerRunLoop(WebCore::WorkerRunLoop *)
0x66166279	 [chrome.dll	 - workerthread.cpp:161]	WebCore::WorkerThread::workerThread()
0x66343637	 [chrome.dll	 - threading.cpp:69]	WTF::threadEntryPoint
0x65dce6bd	 [chrome.dll	 - threadingwin.cpp:213]	WTF::wtfThreadEntryPoint
0x6593abc3	 [chrome.dll	 - threadex.c:348]	_callthreadstartex
0x6593ab87	 [chrome.dll	 - threadex.c:326]	_threadstartex
0x76f23399	 [kernel32.dll	 + 0x00013399]	BaseThreadInitThunk
0x77eb9ef1	 [ntdll.dll	 + 0x00039ef1]	__RtlUserThreadStart
0x77eb9ec4	 [ntdll.dll	 + 0x00039ec4]	_RtlUserThreadStart
Cc: -rolandsteiner@chromium.org
Status: Started
Labels: webkit-id-83104
Project Member Comment 11 by bugdroid1@chromium.org, Apr 4 2012
Labels: -webkit-id-83104 WebKit-ID-83104-NEW
https://bugs.webkit.org/show_bug.cgi?id=83104
Cc: jeffharris@google.com
Cc: michaeln@chromium.org
i'll make the same comment here as i made on a dup of this bug...

Looks like there was a pending transaction at shutdown time.

The cleanup tasks below happen prior to didStopWorkerRunLoop() being invoked. Is it possible that a cleanup task has cleaned up some state that the IDBRequest dtor depends on?

void WorkerRunLoop::run(WorkerContext* context)
{
    RunLoopSetup setup(*this);
    ModePredicate modePredicate(defaultMode());
    MessageQueueWaitResult result;
    do {
        result = runInMode(context, modePredicate, WaitForMessage);
    } while (result != MessageQueueTerminated);
    runCleanupTasks(context);
}

If so, hoisting our calls to PlatformSupport didStartWorkerRunLoop and didStopWorkerRunLoop up into that run method, and calling stop before running cleanup tasks, could help.
Cc: wiltzius@chromium.org
Project Member Comment 16 by bugdroid1@chromium.org, Apr 11 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=131679

------------------------------------------------------------------------
r131679 | dgrogan@chromium.org | Tue Apr 10 17:08:01 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/indexed_db/indexed_db_dispatcher.cc?r1=131679&r2=131678&pathrev=131679

Add DCHECK to ensure IndexedDBDispatcher doesn't get re-created.

This could happen if there are IDB objects that survive the call to
didStopWorkerRunLoop.

BUG= 121734 
TEST=


Review URL: http://codereview.chromium.org/9999035
------------------------------------------------------------------------
Project Member Comment 17 by bugdroid1@chromium.org, Apr 11 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=131683

------------------------------------------------------------------------
r131683 | rsesek@chromium.org | Tue Apr 10 17:39:51 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/indexed_db/indexed_db_dispatcher.cc?r1=131683&r2=131682&pathrev=131683

Revert 131679 - Add DCHECK to ensure IndexedDBDispatcher doesn't get re-created.
Broke content_unittests WebRTCAudioDeviceTest.TestValidOutputRates

This could happen if there are IDB objects that survive the call to
didStopWorkerRunLoop.

BUG= 121734 
TEST=


Review URL: http://codereview.chromium.org/9999035

TBR=dgrogan@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10014040
------------------------------------------------------------------------
Labels: Merge-Requested
The webkit fix landed in http://trac.webkit.org/changeset/113818.  Can we merge it to 19?

Hm, I see it was tagged with Mstone-18.  Is it serious enough to warrant patching into 18?

Labels: -Merge-Requested Merge-Approved
Status: FixUnreleased
We will see the need to merge to m18 if another stable patch comes.
I just noticed in the "Merge instructions for Chrome 19" email:

> Have your change checked into the trunk, and passed through either a Canary build or Dev channel release before asking for merge approval

This fix was just rolled into chrome this morning and hasn't been through a canary or dev release yet. I also got an email from notifier@ saying this merge request was approved improperly, subject:

Merge Approved Improperly(cr): 121734 [FixUnreleased|dgrogan@chromium.org|Mstone-18|SetBy:inferno@chromium.org] - Heap-use-after-free in WebCore::V8AbstractEventListener::~V8AbstractEventListener -  http://crbug.com/121734 

So it seems that my merge request was improper, and inferno's approval was improper. So I'm not going to merge to m19 yet. Let me know if I should merge despite these improprieties.
David, security bugs have blanket approval for merges and we do check with the release managers before doing mass merges. Best to just leave it for now and we will merge to the appropriate branches as needed.
Ok cool, if you guys handle the merging then I'm off the hook.  Thanks!
Project Member Comment 23 by bugdroid1@chromium.org, Apr 13 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=132128

------------------------------------------------------------------------
r132128 | dgrogan@chromium.org | Thu Apr 12 19:16:08 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_thread_impl.cc?r1=132128&r2=132127&pathrev=132128
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/indexed_db/indexed_db_dispatcher.h?r1=132128&r2=132127&pathrev=132128
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/indexed_db/indexed_db_dispatcher.cc?r1=132128&r2=132127&pathrev=132128

Add DCHECK in IndexedDBDispatcher::ThreadSpecificInstance to ensure
IndexedDBDispatcher doesn't get re-created during worker shutdown.

This could happen if there are IDB objects that survive the call to
didStopWorkerRunLoop.

This CL also makes RenderThreadImpl call new IndexedDBDispatcher
instead of IndexedDBDispatcher::ThreadSpecificInstance to avoid
hitting that DCHECK in tests.  WebRTCAudioDeviceTest creates a
RenderThreadImpl object, deletes it, and creates another render
thread object, all on the same thread.

BUG= 121734 
TEST=


Review URL: http://codereview.chromium.org/10052005
------------------------------------------------------------------------
Note that the chrome change doesn't need to be merged to m19's branch, just http://trac.webkit.org/changeset/113818.
Cc: pweis@google.com
I thought this would been merged into m19 by now. Is that going to happen? There's a bug about this affecting internal teams:
http://b.corp.google.com/issue?id=6278119
Please feel free to merge to m19 if you need it early. We were planning to merge this when m19 stable release date comes close. Also please check http://code.google.com/p/chromium/issues/detail?id=124953 with similar/same stack which is still hitting.
Labels: -Mstone-18 -Merge-Approved Mstone-19 Merge-Merged
Merged into m19, branch 1084: http://trac.webkit.org/changeset/115370/
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: CVE-2011-3091
Comment 30 by cdn@chromium.org, May 15 2012
Status: Fixed
Updating status to Fixed on security bugs which were fixed when m19 went to stable.
Project Member Comment 31 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Comment 32 by laforge@google.com, Oct 17 2012
Labels: -Feature-IndexedDB WebKit-Storage-IndexedDB
Project Member Comment 33 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-WebKit -Type-Security -SecSeverity-High -SecImpacts-Stable -Mstone-19 -SecImpacts-Beta -Stability-AddressSanitizer -WebKit-Storage-IndexedDB Cr-Content Security-Impact-Stable Security-Impact-Beta Type-Bug-Security M-19 Security-Severity-High Cr-Content-Storage-IndexedDB Performance-Memory-AddressSanitizer
Project Member Comment 34 by bugdroid1@chromium.org, Mar 13 2013
Labels: Restrict-View-EditIssue
Project Member Comment 35 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member Comment 37 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High Security_Severity-High
Project Member Comment 38 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 39 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Beta Security_Impact-Beta
Project Member Comment 40 by bugdroid1@chromium.org, Apr 1 2013
Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer
Project Member Comment 41 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content Cr-Blink
Project Member Comment 42 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content-Storage-IndexedDB Cr-Blink-Storage-IndexedDB
Cc: falken@chromium.org
Comment 44 by tony@chromium.org, Feb 12 2015
Cc: -tony@chromium.org
Project Member Comment 45 by sheriffbot@chromium.org, Jun 14 2016
Labels: -security_impact-beta
Project Member Comment 46 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 47 by sheriffbot@chromium.org, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment