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

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
link

Issue 121734: Heap-use-after-free in WebCore::V8AbstractEventListener::~V8AbstractEventListener

Reported by infe...@chromium.org, Apr 3 2012 Project Member

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
 

Comment 1 by infe...@chromium.org, Apr 3 2012

Cc: adamk@chromium.org abarth@chromium.org

Comment 2 by infe...@chromium.org, Apr 3 2012

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 ?

Comment 3 by infe...@chromium.org, Apr 3 2012

Cc: -dgro...@chromium.org
Owner: dgro...@chromium.org
Status: Assigned

Comment 4 by jsb...@chromium.org, Apr 3 2012

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.

Comment 5 by jsb...@chromium.org, Apr 3 2012

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.

Comment 6 by jsb...@chromium.org, Apr 3 2012

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

Comment 7 by jsb...@chromium.org, Apr 3 2012

... 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

Comment 8 by rolandsteiner@chromium.org, Apr 4 2012

Cc: -rolandsteiner@chromium.org

Comment 9 by dgro...@chromium.org, Apr 4 2012

Status: Started

Comment 10 by dgro...@chromium.org, Apr 4 2012

Labels: webkit-id-83104

Comment 11 by bugdroid1@chromium.org, Apr 4 2012

Project Member
Labels: -webkit-id-83104 WebKit-ID-83104-NEW
https://bugs.webkit.org/show_bug.cgi?id=83104

Comment 12 by dgro...@chromium.org, Apr 4 2012

Cc: jeffharris@google.com

Comment 13 by dgro...@chromium.org, Apr 4 2012

Cc: michaeln@chromium.org

Comment 14 by michaeln@chromium.org, Apr 5 2012

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.

Comment 15 by dgro...@chromium.org, Apr 5 2012

Cc: wiltzius@chromium.org

Comment 16 by bugdroid1@chromium.org, Apr 11 2012

Project Member
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
------------------------------------------------------------------------

Comment 17 by bugdroid1@chromium.org, Apr 11 2012

Project Member
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
------------------------------------------------------------------------

Comment 18 by dgro...@chromium.org, Apr 11 2012

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?

Comment 19 by infe...@chromium.org, Apr 11 2012

Labels: -Merge-Requested Merge-Approved
Status: FixUnreleased
We will see the need to merge to m18 if another stable patch comes.

Comment 20 by dgro...@chromium.org, Apr 11 2012

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.

Comment 21 by infe...@chromium.org, Apr 11 2012

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.

Comment 22 by dgro...@chromium.org, Apr 11 2012

Ok cool, if you guys handle the merging then I'm off the hook.  Thanks!

Comment 23 by bugdroid1@chromium.org, Apr 13 2012

Project Member
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
------------------------------------------------------------------------

Comment 24 by dgro...@chromium.org, Apr 13 2012

Note that the chrome change doesn't need to be merged to m19's branch, just http://trac.webkit.org/changeset/113818.

Comment 25 by dgro...@chromium.org, Apr 26 2012

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

Comment 26 by infe...@chromium.org, Apr 26 2012

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.

Comment 27 by dgro...@chromium.org, Apr 26 2012

Labels: -Mstone-18 -Merge-Approved Mstone-19 Merge-Merged
Merged into m19, branch 1084: http://trac.webkit.org/changeset/115370/

Comment 28 by scarybea...@gmail.com, May 6 2012

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 29 by scarybea...@gmail.com, May 14 2012

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.

Comment 31 by bugdroid1@chromium.org, Oct 13 2012

Project Member
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

Comment 33 by bugdroid1@chromium.org, Mar 10 2013

Project Member
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

Comment 34 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: Restrict-View-EditIssue

Comment 35 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Comment 36 by scarybea...@gmail.com, Mar 21 2013

Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue

Comment 37 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Severity-High Security_Severity-High

Comment 38 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Stable Security_Impact-Stable

Comment 39 by bugdroid1@chromium.org, Mar 21 2013

Project Member
Labels: -Security-Impact-Beta Security_Impact-Beta

Comment 40 by bugdroid1@chromium.org, Apr 1 2013

Project Member
Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer

Comment 41 by bugdroid1@chromium.org, Apr 6 2013

Project Member
Labels: -Cr-Content Cr-Blink

Comment 42 by bugdroid1@chromium.org, Apr 6 2013

Project Member
Labels: -Cr-Content-Storage-IndexedDB Cr-Blink-Storage-IndexedDB

Comment 43 by dgro...@chromium.org, Feb 12 2015

Cc: falken@chromium.org

Comment 44 by tony@chromium.org, Feb 12 2015

Cc: -tony@chromium.org

Comment 45 by sheriffbot@chromium.org, Jun 14 2016

Project Member
Labels: -security_impact-beta

Comment 46 by sheriffbot@chromium.org, Oct 1 2016

Project Member
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

Comment 47 by sheriffbot@chromium.org, Oct 2 2016

Project Member
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

Comment 48 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Comment 49 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment