New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 121734 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
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

Issue description

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
Labels: CVE_description-submitted

Sign in to add a comment