108538 2013-01-31 15:12:13 -0800 Dubious cast from HTMLCollection to HTMLAllCollection 2013-04-16 13:28:29 -0700 1 1 1 Unclassified Security Security Other All All RESOLVED FIXED InRadar P2 Trivial --- 1 tsepez tsepez abarth cevans jschuh ojan.autocc tsepez webkit-bug-importer webkit.review.bot oldest_to_newest 822008 0 tsepez 2013-01-31 15:12:13 -0800 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. 822022 1 tsepez 2013-01-31 15:19:29 -0800 The others look to be safe. Removing flag. 822146 2 185900 tsepez 2013-01-31 17:05:05 -0800 Created attachment 185900 Patch. 822457 3 185900 webkit.review.bot 2013-02-01 00:53:34 -0800 Comment on attachment 185900 Patch. Clearing flags on attachment: 185900 Committed r141556: <http://trac.webkit.org/changeset/141556> 822458 4 webkit.review.bot 2013-02-01 00:53:38 -0800 All reviewed patches have been landed. Closing bug. 876807 5 webkit-bug-importer 2013-04-16 13:28:29 -0700 <rdar://problem/13666402> 185900 2013-01-31 17:05:05 -0800 2013-02-01 00:53:34 -0800 Patch. patch_108538.txt text/plain 3817 tsepez SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE0MTUwNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI3IEBACisyMDEzLTAxLTMxICBUb20gU2Vw ZXogIDx0c2VwZXpAY2hyb21pdW0ub3JnPgorCisgICAgICAgIER1YmlvdXMgY2FzdCBmcm9tIEhU TUxDb2xsZWN0aW9uIHRvIEhUTUxBbGxDb2xsZWN0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDg1MzgKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBQYXRjaCBpcyB0ZXN0ZWQgYnkgZW5hYmxpbmcgVjgg YmluZGluZyBpbnRlZ3JpdHkgb24gSFRNTEFsbENvbGxlY3Rpb24gYW5kCisgICAgICAgIHJ1bm5p bmcgdGhlIGV4aXN0aW5nIHRlc3RzIHdpdGhvdXQgaW50cm9kdWNpbmcgbmV3IGNyYXNoZXMuCisK KyAgICAgICAgKiBkb20vRG9jdW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6RG9jdW1lbnQ6 OmFsbCk6CisgICAgICAgIFBhc3MgY29ycmVjdCB0eXBlIHRvIHRlbXBsYXRlLgorICAgICAgICAK KyAgICAgICAgKiBodG1sL0hUTUxBbGxDb2xsZWN0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkhUTUxBbGxDb2xsZWN0aW9uOjpjcmVhdGUpOgorICAgICAgICAoV2ViQ29yZTo6SFRNTEFsbENv bGxlY3Rpb246OkhUTUxBbGxDb2xsZWN0aW9uKToKKyAgICAgICAgKiBodG1sL0hUTUxBbGxDb2xs ZWN0aW9uLmg6CisgICAgICAgIChIVE1MQWxsQ29sbGVjdGlvbik6CisgICAgICAgIE1ha2UgY3Jl YXRlKCkgbWV0aG9kIGFyZ3VtZW50cyBjb21wYXRpYmxlIHdpdGggdGVtcGxhdGUgYWJvdmUuCisK KyAgICAgICAgKiBodG1sL0hUTUxBbGxDb2xsZWN0aW9uLmlkbDoKKyAgICAgICAgRW5hYmxlIGJp bmRpbmcgaW50ZWdyaXR5LgorCiAyMDEzLTAxLTMxICBKdWxpZW4gQ2hhZmZyYWl4ICA8amNoYWZm cmFpeEB3ZWJraXQub3JnPgogCiAgICAgICAgIFtDU1MgR3JpZCBMYXlvdXRdIFN1cHBvcnQgaW1w bGljaXQgcm93cyBhbmQgY29sdW1ucwpJbmRleDogU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50 LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuY3BwCShyZXZp c2lvbiAxNDE1MDUpCisrKyBTb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC00MzY5LDcgKzQzNjksNyBAQCBQYXNzUmVmUHRyPEhUTUxDb2xsZWN0aW9uPiBE b2N1bWVudDo6YW5jCiAKIFBhc3NSZWZQdHI8SFRNTENvbGxlY3Rpb24+IERvY3VtZW50OjphbGwo KQogewotICAgIHJldHVybiBlbnN1cmVDYWNoZWRDb2xsZWN0aW9uKERvY0FsbCk7CisgICAgcmV0 dXJuIGVuc3VyZVJhcmVEYXRhKCktPmVuc3VyZU5vZGVMaXN0cygpLT5hZGRDYWNoZVdpdGhBdG9t aWNOYW1lPEhUTUxBbGxDb2xsZWN0aW9uPih0aGlzLCBEb2NBbGwpOwogfQogCiBQYXNzUmVmUHRy PEhUTUxDb2xsZWN0aW9uPiBEb2N1bWVudDo6d2luZG93TmFtZWRJdGVtcyhjb25zdCBBdG9taWNT dHJpbmcmIG5hbWUpCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxBbGxDb2xsZWN0aW9u LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxBbGxDb2xsZWN0aW9u LmNwcAkocmV2aXNpb24gMTQxNTA1KQorKysgU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQWxsQ29s bGVjdGlvbi5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMwLDEzICszMCwxMyBAQAogCiBuYW1lc3Bh Y2UgV2ViQ29yZSB7CiAKLVBhc3NSZWZQdHI8SFRNTEFsbENvbGxlY3Rpb24+IEhUTUxBbGxDb2xs ZWN0aW9uOjpjcmVhdGUoRG9jdW1lbnQqIGRvY3VtZW50KQorUGFzc1JlZlB0cjxIVE1MQWxsQ29s bGVjdGlvbj4gSFRNTEFsbENvbGxlY3Rpb246OmNyZWF0ZShOb2RlKiBub2RlLCBDb2xsZWN0aW9u VHlwZSB0eXBlKQogewotICAgIHJldHVybiBhZG9wdFJlZihuZXcgSFRNTEFsbENvbGxlY3Rpb24o ZG9jdW1lbnQpKTsKKyAgICByZXR1cm4gYWRvcHRSZWYobmV3IEhUTUxBbGxDb2xsZWN0aW9uKG5v ZGUsIHR5cGUpKTsKIH0KIAotSFRNTEFsbENvbGxlY3Rpb246OkhUTUxBbGxDb2xsZWN0aW9uKERv Y3VtZW50KiBkb2N1bWVudCkKLSAgICA6IEhUTUxDb2xsZWN0aW9uKGRvY3VtZW50LCBEb2NBbGws IERvZXNOb3RPdmVycmlkZUl0ZW1BZnRlcikKK0hUTUxBbGxDb2xsZWN0aW9uOjpIVE1MQWxsQ29s bGVjdGlvbihOb2RlKiBub2RlLCBDb2xsZWN0aW9uVHlwZSB0eXBlKQorICAgIDogSFRNTENvbGxl Y3Rpb24obm9kZSwgdHlwZSwgRG9lc05vdE92ZXJyaWRlSXRlbUFmdGVyKQogewogfQogCkluZGV4 OiBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxBbGxDb2xsZWN0aW9uLmgKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQWxsQ29sbGVjdGlvbi5oCShyZXZpc2lvbiAxNDE1MDUp CisrKyBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxBbGxDb2xsZWN0aW9uLmgJKHdvcmtpbmcgY29w eSkKQEAgLTMyLDEzICszMiwxMyBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKIGNsYXNzIEhUTUxB bGxDb2xsZWN0aW9uIDogcHVibGljIEhUTUxDb2xsZWN0aW9uIHsKIHB1YmxpYzoKLSAgICBzdGF0 aWMgUGFzc1JlZlB0cjxIVE1MQWxsQ29sbGVjdGlvbj4gY3JlYXRlKERvY3VtZW50Kik7CisgICAg c3RhdGljIFBhc3NSZWZQdHI8SFRNTEFsbENvbGxlY3Rpb24+IGNyZWF0ZShOb2RlKiwgQ29sbGVj dGlvblR5cGUpOwogICAgIHZpcnR1YWwgfkhUTUxBbGxDb2xsZWN0aW9uKCk7CiAKICAgICBOb2Rl KiBuYW1lZEl0ZW1XaXRoSW5kZXgoY29uc3QgQXRvbWljU3RyaW5nJiBuYW1lLCB1bnNpZ25lZCBp bmRleCkgY29uc3Q7CiAKIHByaXZhdGU6Ci0gICAgSFRNTEFsbENvbGxlY3Rpb24oRG9jdW1lbnQq KTsKKyAgICBIVE1MQWxsQ29sbGVjdGlvbihOb2RlKiwgQ29sbGVjdGlvblR5cGUpOwogfTsKIAog fSAvLyBuYW1lc3BhY2UgV2ViQ29yZQpJbmRleDogU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQWxs Q29sbGVjdGlvbi5pZGwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQWxs Q29sbGVjdGlvbi5pZGwJKHJldmlzaW9uIDE0MTUwNSkKKysrIFNvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTEFsbENvbGxlY3Rpb24uaWRsCSh3b3JraW5nIGNvcHkpCkBAIC0zMCw3ICszMCw2IEBACiAg ICAgTWFzcXVlcmFkZXNBc1VuZGVmaW5lZCwKICAgICBHZW5lcmF0ZUlzUmVhY2hhYmxlPUltcGxP d25lck5vZGVSb290LAogICAgIFY4RGVwZW5kZW50TGlmZXRpbWUsCi0gICAgVjhTa2lwVlRhYmxl VmFsaWRhdGlvbgogXSBpbnRlcmZhY2UgSFRNTEFsbENvbGxlY3Rpb24gewogICAgIHJlYWRvbmx5 IGF0dHJpYnV0ZSB1bnNpZ25lZCBsb25nIGxlbmd0aDsKICAgICBbQ3VzdG9tXSBOb2RlIGl0ZW0o aW4gW09wdGlvbmFsPURlZmF1bHRJc1VuZGVmaW5lZF0gdW5zaWduZWQgbG9uZyBpbmRleCk7Cg==