| Summary: | Dubious cast from HTMLCollection to HTMLAllCollection | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Security | Reporter: | Thomas Sepez <tsepez> | ||||
| Component: | Security | Assignee: | Thomas Sepez <tsepez> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Trivial | CC: | abarth, cevans, jschuh, ojan.autocc, tsepez, webkit-bug-importer, webkit.review.bot | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | Other | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
The others look to be safe. Removing flag. Created attachment 185900 [details]
Patch.
Comment on attachment 185900 [details] Patch. Clearing flags on attachment: 185900 Committed r141556: <http://trac.webkit.org/changeset/141556> All reviewed patches have been landed. Closing bug. |
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 } 84 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.