46279 2010-09-22 10:58:34 -0700 Random plug-in cleanups 2010-09-22 11:16:37 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 andersca andersca darin webkit.review.bot oldest_to_newest 283054 0 andersca 2010-09-22 10:58:34 -0700 Random plug-in cleanups 283055 1 68395 andersca 2010-09-22 11:00:35 -0700 Created attachment 68395 Patch 283058 2 webkit.review.bot 2010-09-22 11:04:20 -0700 Attachment 68395 did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/Plugins/Plugin.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style. 283060 3 68395 darin 2010-09-22 11:05:46 -0700 Comment on attachment 68395 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=68395&action=review > WebKit2/WebProcess/Plugins/Plugin.cpp:48 > + parameters.url = KURL(ParsedURLString, urlString); Are you sure this is the constructor we want to use here? This is normally only suitable if we know that the URL is already parsed by KURL and the string came out of KURL. Is that going to be true. Further, is this consistent with the “don’t trust the other process” philosophy? > WebKit2/WebProcess/Plugins/Plugin.cpp:57 > + if (!decoder->decode(parameters.names)) > + return false; > + if (!decoder->decode(parameters.values)) > + return false; > + if (!decoder->decode(parameters.mimeType)) > + return false; > + if (!decoder->decode(parameters.loadManually)) > + return false; Do these functions normally check for validity? For example, are the names and values arrays the same length? Are they all strings in those arrays? Are there any invalid values for anything? 283071 4 andersca 2010-09-22 11:12:39 -0700 (In reply to comment #3) > (From update of attachment 68395 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68395&action=review > > > WebKit2/WebProcess/Plugins/Plugin.cpp:48 > > + parameters.url = KURL(ParsedURLString, urlString); > > Are you sure this is the constructor we want to use here? This is normally only suitable if we know that the URL is already parsed by KURL and the string came out of KURL. Is that going to be true. Further, is this consistent with the “don’t trust the other process” philosophy? > I hadn't thought about the possibility that the URLs might be invalid because they came from a compromised web process. I think the best way to solve this is to add an argument coder for KURL which checks that the URLs are valid, and I'll do this in a followup patch. > > WebKit2/WebProcess/Plugins/Plugin.cpp:57 > > + if (!decoder->decode(parameters.names)) > > + return false; > > + if (!decoder->decode(parameters.values)) > > + return false; > > + if (!decoder->decode(parameters.mimeType)) > > + return false; > > + if (!decoder->decode(parameters.loadManually)) > > + return false; > > Do these functions normally check for validity? For example, are the names and values arrays the same length? Are they all strings in those arrays? Are there any invalid values for anything? They check that the strings are valid, but not that the number of names matches the number of values. I'll fix that. 283079 5 andersca 2010-09-22 11:16:37 -0700 Committed r68058: <http://trac.webkit.org/changeset/68058> 68395 2010-09-22 11:00:35 -0700 2010-09-22 11:05:46 -0700 Patch bug-46279-20100922110033.patch text/plain 5645 andersca ZGlmZiAtLWdpdCBhL1dlYktpdDIvQ2hhbmdlTG9nIGIvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXgg MWM5N2IwZTU2ZGRhNGY5MTlhZThmZjJlM2YwNDc0NDE4YzI4NjNiZC4uZjczOGY3ODVlZWI5MDkw ZTYxNGI0YmRlZjM3OTdiMmIwOGQxMzI3NCAxMDA2NDQKLS0tIGEvV2ViS2l0Mi9DaGFuZ2VMb2cK KysrIGIvV2ViS2l0Mi9DaGFuZ2VMb2cKQEAgLTEsNSArMSwyNyBAQAogMjAxMC0wOS0yMiAgQW5k ZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJhbmRvbSBwbHVnLWluIGNsZWFudXBzCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NjI3OQorCisgICAg ICAgICogV2ViUHJvY2Vzcy9QbHVnaW5zL1BsdWdpbi5jcHA6CisgICAgICAgIChXZWJLaXQ6OlBs dWdpbjo6UGFyYW1ldGVyczo6ZW5jb2RlKToKKyAgICAgICAgKFdlYktpdDo6UGx1Z2luOjpQYXJh bWV0ZXJzOjpkZWNvZGUpOgorICAgICAgICBBZGQgQ29yZUlQQyBjb2Rpbmcgc3VwcG9ydCBmb3Ig dGhlIFBsdWdpbjo6UGFyYW1ldGVycyBzdHJ1Y3QuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1Bs dWdpbnMvUGx1Z2luQ29udHJvbGxlci5oOgorICAgICAgICBBZGQgcGx1Z2luUHJvY2Vzc0NyYXNo ZWQgcHVyZSB2aXJ0dWFsIG1lbWJlciBmdW5jdGlvbi4KKworICAgICAgICAqIFdlYlByb2Nlc3Mv UGx1Z2lucy9QbHVnaW5WaWV3LmNwcDoKKyAgICAgICAgKFdlYktpdDo6UGx1Z2luVmlldzo6aGFu ZGxlRXZlbnQpOgorICAgICAgICBoYW5kbGVFdmVudCBjYW4gYmUgY2FsbGVkIHdoZW4gbV9wbHVn aW4gaXMgbnVsbC4KKworICAgICAgICAoV2ViS2l0OjpQbHVnaW5WaWV3OjpwbHVnaW5Qcm9jZXNz Q3Jhc2hlZCk6CisgICAgICAgIEFzayB0aGUgcmVuZGVyZXIgdG8gc2hvdyB0aGUgY3Jhc2hlZCBw bHVnLWluIGluZGljYXRvci4KKyAgICAgICAgCisyMDEwLTA5LTIyICBBbmRlcnMgQ2FybHNzb24g IDxhbmRlcnNjYUBhcHBsZS5jb20+CisKICAgICAgICAgUmV2aWV3ZWQgYnkgSm9obiBTdWxsaXZh bi4KIAogICAgICAgICBEb24ndCBsaXN0ZW4gZm9yIHNvbWUgd2luZG93IG5vdGlmaWNhdGlvbnMg b24gYWxsIHdpbmRvd3MKZGlmZiAtLWdpdCBhL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVnaW5zL1Bs dWdpbi5jcHAgYi9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW4uY3BwCmluZGV4IDEy NjUwZTBhYzRiYTgyYmVkZTExN2RjN2RlOGIxZmQ0NzAwZjNlMDYuLjFlNzAwZWRlZjA4NzBhMGFj YTZiMTA1NDI0MTEzNWRiZjk2ZmY3Y2MgMTAwNjQ0Ci0tLSBhL1dlYktpdDIvV2ViUHJvY2Vzcy9Q bHVnaW5zL1BsdWdpbi5jcHAKKysrIGIvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2lu LmNwcApAQCAtMjUsMTAgKzI1LDQwIEBACiAKICNpbmNsdWRlICJQbHVnaW4uaCIKIAorI2luY2x1 ZGUgIldlYkNvcmVBcmd1bWVudENvZGVycy5oIgorCiB1c2luZyBuYW1lc3BhY2UgV2ViQ29yZTsK IAogbmFtZXNwYWNlIFdlYktpdCB7CiAKK3ZvaWQgUGx1Z2luOjpQYXJhbWV0ZXJzOjplbmNvZGUo Q29yZUlQQzo6QXJndW1lbnRFbmNvZGVyKiBlbmNvZGVyKSBjb25zdAoreworICAgIGVuY29kZXIt PmVuY29kZSh1cmwuc3RyaW5nKCkpOworICAgIGVuY29kZXItPmVuY29kZShuYW1lcyk7CisgICAg ZW5jb2Rlci0+ZW5jb2RlKHZhbHVlcyk7CisgICAgZW5jb2Rlci0+ZW5jb2RlKG1pbWVUeXBlKTsK KyAgICBlbmNvZGVyLT5lbmNvZGUobG9hZE1hbnVhbGx5KTsKK30KKworYm9vbCBQbHVnaW46OlBh cmFtZXRlcnM6OmRlY29kZShDb3JlSVBDOjpBcmd1bWVudERlY29kZXIqIGRlY29kZXIsIFBhcmFt ZXRlcnMmIHBhcmFtZXRlcnMpCit7CisgICAgU3RyaW5nIHVybFN0cmluZzsKKyAgICBpZiAoIWRl Y29kZXItPmRlY29kZSh1cmxTdHJpbmcpKQorICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgcGFy YW1ldGVycy51cmwgPSBLVVJMKFBhcnNlZFVSTFN0cmluZywgdXJsU3RyaW5nKTsKKworICAgIGlm ICghZGVjb2Rlci0+ZGVjb2RlKHBhcmFtZXRlcnMubmFtZXMpKQorICAgICAgICByZXR1cm4gZmFs c2U7CisgICAgaWYgKCFkZWNvZGVyLT5kZWNvZGUocGFyYW1ldGVycy52YWx1ZXMpKQorICAgICAg ICByZXR1cm4gZmFsc2U7CisgICAgaWYgKCFkZWNvZGVyLT5kZWNvZGUocGFyYW1ldGVycy5taW1l VHlwZSkpCisgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICBpZiAoIWRlY29kZXItPmRlY29kZShw YXJhbWV0ZXJzLmxvYWRNYW51YWxseSkpCisgICAgICAgIHJldHVybiBmYWxzZTsKKworICAgIHJl dHVybiB0cnVlOworfQorCiBQbHVnaW46OlBsdWdpbigpCiB7CiB9CmRpZmYgLS1naXQgYS9XZWJL aXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW4uaCBiL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVn aW5zL1BsdWdpbi5oCmluZGV4IGJmYWJhNGViNDVkNDhjYzQxYWRlNThjZjFkNTkwZTY3ZDI1MDJj NWUuLmZjMDhiZTczOGI1ZWMwNjQ4YTI4MzBhN2U4MzljYTQ5OTNmZjc1ZmUgMTAwNjQ0Ci0tLSBh L1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVnaW5zL1BsdWdpbi5oCisrKyBiL1dlYktpdDIvV2ViUHJv Y2Vzcy9QbHVnaW5zL1BsdWdpbi5oCkBAIC0zMyw2ICszMywxMSBAQAogCiBzdHJ1Y3QgTlBPYmpl Y3Q7CiAKK25hbWVzcGFjZSBDb3JlSVBDIHsKKyAgICBjbGFzcyBBcmd1bWVudEVuY29kZXI7Cisg ICAgY2xhc3MgQXJndW1lbnREZWNvZGVyOworfQorCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAg Y2xhc3MgR3JhcGhpY3NDb250ZXh0OwogICAgIGNsYXNzIEludFJlY3Q7CkBAIC01Myw2ICs1OCw5 IEBAIHB1YmxpYzoKICAgICAgICAgVmVjdG9yPFN0cmluZz4gdmFsdWVzOwogICAgICAgICBTdHJp bmcgbWltZVR5cGU7CiAgICAgICAgIGJvb2wgbG9hZE1hbnVhbGx5OworCisgICAgICAgIHZvaWQg ZW5jb2RlKENvcmVJUEM6OkFyZ3VtZW50RW5jb2RlciopIGNvbnN0OworICAgICAgICBzdGF0aWMg Ym9vbCBkZWNvZGUoQ29yZUlQQzo6QXJndW1lbnREZWNvZGVyKiwgUGFyYW1ldGVycyYpOwogICAg IH07CiAKICAgICB2aXJ0dWFsIH5QbHVnaW4oKTsKZGlmZiAtLWdpdCBhL1dlYktpdDIvV2ViUHJv Y2Vzcy9QbHVnaW5zL1BsdWdpbkNvbnRyb2xsZXIuaCBiL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVn aW5zL1BsdWdpbkNvbnRyb2xsZXIuaAppbmRleCA0NTg4M2RjZWQxNzNkMDgwYmY0ZmJmYmMxMWZm OGZjNWQzMjI0ZWE4Li4wNDQ4ZWRlZWE3NTY5Y2U2NjBmMjliZGRlNzU2Y2NkMjFhM2U2MTNjIDEw MDY0NAotLS0gYS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5Db250cm9sbGVyLmgK KysrIGIvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2luQ29udHJvbGxlci5oCkBAIC04 MCw2ICs4MCw5IEBAIHB1YmxpYzoKICAgICB2aXJ0dWFsIGJvb2wgaXNBY2NlbGVyYXRlZENvbXBv c2l0aW5nRW5hYmxlZCgpID0gMDsKICNlbmRpZgogCisgICAgLy8gVGVsbHMgdGhlIGNvbnRyb2xs ZXIgdGhhdCB0aGUgcGx1Zy1pbiBwcm9jZXNzIGhhcyBjcmFzaGVkLgorICAgIHZpcnR1YWwgdm9p ZCBwbHVnaW5Qcm9jZXNzQ3Jhc2hlZCgpID0gMDsKKyAgICAKIHByb3RlY3RlZDoKICAgICB2aXJ0 dWFsIH5QbHVnaW5Db250cm9sbGVyKCkgeyB9CiB9OwpkaWZmIC0tZ2l0IGEvV2ViS2l0Mi9XZWJQ cm9jZXNzL1BsdWdpbnMvUGx1Z2luVmlldy5jcHAgYi9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lu cy9QbHVnaW5WaWV3LmNwcAppbmRleCBkN2JiMGVhMmFlOWMwMzRlYzljN2ZkOTZjMmU2ZjZkNTY5 MjIzZTllLi4yNmUzOWIwNmFlYWZmZWYyNDViNjdiNTFkZjUxNmNhODFmZmYwNjZlIDEwMDY0NAot LS0gYS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5WaWV3LmNwcAorKysgYi9XZWJL aXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5WaWV3LmNwcApAQCAtNDAsNiArNDAsNyBAQAog I2luY2x1ZGUgPFdlYkNvcmUvSFRNTFBsdWdJbkVsZW1lbnQuaD4KICNpbmNsdWRlIDxXZWJDb3Jl L0hvc3RXaW5kb3cuaD4KICNpbmNsdWRlIDxXZWJDb3JlL05ldHNjYXBlUGx1Z0luU3RyZWFtTG9h ZGVyLmg+CisjaW5jbHVkZSA8V2ViQ29yZS9SZW5kZXJFbWJlZGRlZE9iamVjdC5oPgogI2luY2x1 ZGUgPFdlYkNvcmUvUmVuZGVyTGF5ZXIuaD4KICNpbmNsdWRlIDxXZWJDb3JlL1Njcm9sbFZpZXcu aD4KICNpbmNsdWRlIDxXZWJDb3JlL1NldHRpbmdzLmg+CkBAIC00NTcsNiArNDU4LDkgQEAgdm9p ZCBQbHVnaW5WaWV3OjpzZXRQYXJlbnQoU2Nyb2xsVmlldyogc2Nyb2xsVmlldykKIAogdm9pZCBQ bHVnaW5WaWV3OjpoYW5kbGVFdmVudChFdmVudCogZXZlbnQpCiB7CisgICAgaWYgKCFtX3BsdWdp bikKKyAgICAgICAgcmV0dXJuOworCiAgICAgY29uc3QgV2ViRXZlbnQqIGN1cnJlbnRFdmVudCA9 IFdlYlBhZ2U6OmN1cnJlbnRFdmVudCgpOwogICAgIGlmICghY3VycmVudEV2ZW50KQogICAgICAg ICByZXR1cm47CkBAIC04MDUsNiArODA5LDE0IEBAIGJvb2wgUGx1Z2luVmlldzo6aXNBY2NlbGVy YXRlZENvbXBvc2l0aW5nRW5hYmxlZCgpCiAgICAgcmV0dXJuIHNldHRpbmdzLT5hY2NlbGVyYXRl ZENvbXBvc2l0aW5nRW5hYmxlZCgpOwogfQogCit2b2lkIFBsdWdpblZpZXc6OnBsdWdpblByb2Nl c3NDcmFzaGVkKCkKK3sKKyAgICBpZiAoUmVuZGVyRW1iZWRkZWRPYmplY3QqIHJlbmRlcmVyID0g dG9SZW5kZXJFbWJlZGRlZE9iamVjdChtX3BsdWdpbkVsZW1lbnQtPnJlbmRlcmVyKCkpKQorICAg ICAgICByZW5kZXJlci0+c2V0U2hvd3NDcmFzaGVkUGx1Z2luSW5kaWNhdG9yKCk7CisgICAgCisg ICAgaW52YWxpZGF0ZVJlY3QoZnJhbWVSZWN0KCkpOworfQorCiB2b2lkIFBsdWdpblZpZXc6OmRp ZEZpbmlzaExvYWQoV2ViRnJhbWUqIHdlYkZyYW1lKQogewogICAgIFJlZlB0cjxVUkxSZXF1ZXN0 PiByZXF1ZXN0ID0gbV9wZW5kaW5nRnJhbWVMb2Fkcy50YWtlKHdlYkZyYW1lKTsKZGlmZiAtLWdp dCBhL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVnaW5zL1BsdWdpblZpZXcuaCBiL1dlYktpdDIvV2Vi UHJvY2Vzcy9QbHVnaW5zL1BsdWdpblZpZXcuaAppbmRleCAzODM4YzFkYjljMmVhZTdkNWNmNjE5 MjAzZDU3YWZkYTIxMzIzYzU4Li44OGI0MzE4ZjU0ZDcwMjBmMTdjM2I3NWNjZDAxMDQ3NjRhZTI5 NzM3IDEwMDY0NAotLS0gYS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5WaWV3LmgK KysrIGIvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2luVmlldy5oCkBAIC0xMjMsNiAr MTIzLDcgQEAgcHJpdmF0ZToKICAgICB2aXJ0dWFsIGJvb2wgZXZhbHVhdGUoTlBPYmplY3QqLCBj b25zdCBTdHJpbmcmc2NyaXB0U3RyaW5nLCBOUFZhcmlhbnQqIHJlc3VsdCwgYm9vbCBhbGxvd1Bv cHVwcyk7CiAgICAgdmlydHVhbCB2b2lkIHNldFN0YXR1c2JhclRleHQoY29uc3QgU3RyaW5nJik7 CiAgICAgdmlydHVhbCBib29sIGlzQWNjZWxlcmF0ZWRDb21wb3NpdGluZ0VuYWJsZWQoKTsKKyAg ICB2aXJ0dWFsIHZvaWQgcGx1Z2luUHJvY2Vzc0NyYXNoZWQoKTsKIAogICAgIC8vIFdlYkZyYW1l OjpMb2FkTGlzdGVuZXIKICAgICB2aXJ0dWFsIHZvaWQgZGlkRmluaXNoTG9hZChXZWJGcmFtZSop Owo=