Bug 108538 - Dubious cast from HTMLCollection to HTMLAllCollection
Summary: Dubious cast from HTMLCollection to HTMLAllCollection
Alias: None
Product: Security
Classification: Unclassified
Component: Security (show other bugs)
Version: Other
Hardware: All All
: P2 Trivial
Assignee: Thomas Sepez
Keywords: InRadar
Depends on:
Reported: 2013-01-31 15:12 PST by Thomas Sepez
Modified: 2013-04-16 13:28 PDT (History)
7 users (show)

See Also:

Patch. (3.73 KB, patch)
2013-01-31 17:05 PST, Thomas Sepez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Sepez 2013-01-31 15:12:13 PST
This was noticed as part of the V8 Bindings Integrity project.  The issue does not appear to manifest itself at runtime as there are no additional members in an instance of HTMLAllCollection beyond those found in HTMLCollection.  Nonetheless, something could change down the road and then it won't be OK.

The cast is occurring in the V8 bindings code, however it is not V8 that is at fault here:

71	v8::Handle<v8::Object> wrap(HTMLCollection* impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
72	{
73	    ASSERT(impl);
74	    switch (impl->type()) { 
75	    case FormControls:
76	        return wrap(static_cast<HTMLFormControlsCollection*>(impl), creationContext, isolate);
77	    case SelectOptions:
78	        return wrap(static_cast<HTMLOptionsCollection*>(impl), creationContext, isolate);
79	    case DocAll:
80	        return wrap(static_cast<HTMLAllCollection*>(impl), creationContext, isolate);
81	    default:
82	        break;
83	    }
85	    return V8HTMLCollection::createWrapper(impl, creationContext, isolate);
86	}

HTMLCollections that return a type() of "DocALL" thus have the right to be cast to the HTMLAllCollection subclass.
Yet in WebCore::Document::all(), there is a call to ensureCachedCollection(DocAll),  
which in turn calls:
ensureRareData()->ensureNodeLists()->addCacheWithAtomicName<HTMLCollection>(this, type)
which in turn calls
RefPtr<T> list = T::create(node, collectionType)
where T is HTMLCollection.

Sooo ... an object of the wrong subclass gets created for the given collectionType.

Filing under security as there are similar cases which may not be benign -- but I've not run them to ground yet.
Comment 1 Thomas Sepez 2013-01-31 15:19:29 PST
The others look to be safe.  Removing flag.
Comment 2 Thomas Sepez 2013-01-31 17:05:05 PST
Created attachment 185900 [details]
Comment 3 WebKit Review Bot 2013-02-01 00:53:34 PST
Comment on attachment 185900 [details]

Clearing flags on attachment: 185900

Committed r141556: <http://trac.webkit.org/changeset/141556>
Comment 4 WebKit Review Bot 2013-02-01 00:53:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2013-04-16 13:28:29 PDT