27639 2009-07-23 22:36:43 -0700 XSS Auditor interferes with -[WebView stringByEvaluatingJavaScriptFromString:] 2011-05-27 16:49:45 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED XSSAuditor P2 Normal --- 1 mrowe abarth abarth dbates oldest_to_newest 134365 0 mrowe 2009-07-23 22:36:43 -0700 I tracked down an assertion failure in DumpRenderTree this afternoon that is caused by the XSS auditor causing the following API invocation to fail: [webView stringByEvaluatingJavaScriptFromString:@"0"]; With the XSS auditor enabled, this returns the empty string rather than "0" as expected. This was being tripped over by DumpRenderTree on machines that had never run the layout tests before due to the default preferences being incorrectly configured (<http://trac.webkit.org/changeset/46304>), but it seems undesirable for the XSS auditor to have any effect on these sorts of APIs. 134367 1 abarth 2009-07-23 22:41:50 -0700 We should actually do two things here: 1) If the canonical string is empty, we should allow the script. 2) We should white list the API to never be blocked. 134475 2 abarth 2009-07-24 02:26:44 -0700 Assigned to myself to resolution in the morning. 134613 3 33457 dbates 2009-07-24 11:52:00 -0700 Created attachment 33457 Patch to check that canonicalString is not empty This patch does not include the white list functionality. 134625 4 33457 abarth 2009-07-24 12:27:54 -0700 Comment on attachment 33457 Patch to check that canonicalString is not empty Thank. We just need a test. :) 134626 5 33457 abarth 2009-07-24 12:27:56 -0700 Comment on attachment 33457 Patch to check that canonicalString is not empty Thanks. We just need a test. :) 134634 6 33464 dbates 2009-07-24 12:52:21 -0700 Created attachment 33464 Patch with test Added test. 134639 7 33464 mrowe 2009-07-24 13:02:52 -0700 Comment on attachment 33464 Patch with test Do we also plan on adding a more explicit mechanism for preventing legitimate JavaScript-related WebKit API calls from being touched by the XSS auditor? 134640 8 33464 abarth 2009-07-24 13:02:54 -0700 Comment on attachment 33464 Patch with test A patch of beauty. 134641 9 abarth 2009-07-24 13:03:48 -0700 (In reply to comment #7) > (From update of attachment 33464 [details]) > Do we also plan on adding a more explicit mechanism for preventing legitimate > JavaScript-related WebKit API calls from being touched by the XSS auditor? Yes. We also want to do this patch and it's easier. 134642 10 dbates 2009-07-24 13:09:51 -0700 I have not looked into this, but we may need to be mindful of plugins which use the API to evaluate JavaScript scripts. In which case, we wouldn't want to disable the filter. (In reply to comment #9) > (In reply to comment #7) > > (From update of attachment 33464 [details] [details]) > > Do we also plan on adding a more explicit mechanism for preventing legitimate > > JavaScript-related WebKit API calls from being touched by the XSS auditor? > > Yes. We also want to do this patch and it's easier. 134643 11 abarth 2009-07-24 13:13:16 -0700 (In reply to comment #10) > I have not looked into this, but we may need to be mindful of plugins which use > the API to evaluate JavaScript scripts. In which case, we wouldn't want to > disable the filter. We should look at all the clients of evaluateScript and think about whether they need XSS filtering. 134652 12 abarth 2009-07-24 13:59:56 -0700 Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/http/tests/security/xssAuditor/script-tag-safe-expected.txt A LayoutTests/http/tests/security/xssAuditor/script-tag-safe.html M WebCore/ChangeLog M WebCore/page/XSSAuditor.cpp Committed r46372 M WebCore/ChangeLog M WebCore/page/XSSAuditor.cpp A LayoutTests/http/tests/security/xssAuditor/script-tag-safe-expected.txt A LayoutTests/http/tests/security/xssAuditor/script-tag-safe.html M LayoutTests/ChangeLog r46372 = 20701f1ff156a5bec23db64671d4cd7556a2cde8 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46372 134667 13 abarth 2009-07-24 14:41:12 -0700 I think we should keep this bug open to do the whitelisting part. (Or we could file another bug, I guess.) 134668 14 33464 abarth 2009-07-24 14:41:31 -0700 Comment on attachment 33464 Patch with test Removing from commit queue. 411712 15 abarth 2011-05-27 16:49:45 -0700 I'd be surprised of this was still an issue given the new XSS auditor design. If there's something to fix here, please re-open with an example! Thx. 33457 2009-07-24 11:52:00 -0700 2009-07-24 12:52:21 -0700 Patch to check that canonicalString is not empty Bug27639_1.patch text/plain 1893 dbates SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NjM2NykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDktMDctMjQgIERhbmllbCBCYXRlcyAgPGRiYXRlc0BpbnR1ZGF0 YS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisgICAgICAgIAor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mjc2MzkKKwor ICAgICAgICBGaXhlcyBmYWxzZSBwb3NpdGl2ZXMgd2hlbiBldmFsdWF0aW5nIGNlcnRhaW4gc3Ry aW5ncyB0aGF0IG9ubHkgY29udGFpbiAKKyAgICAgICAgbm9uLWNhbm9uaWNhbCBjaGFyYWN0ZXJz LgorCisgICAgICAgICogcGFnZS9YU1NBdWRpdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Omlz Tm9uQ2Fub25pY2FsQ2hhcmFjdGVyKToKKyAgICAgICAgKFdlYkNvcmU6OlhTU0F1ZGl0b3I6OmZp bmRJblJlcXVlc3QpOgorCiAyMDA5LTA3LTI0ICBKaWFuIExpICA8amlhbmxpQGNocm9taXVtLm9y Zz4KIAogICAgICAgICBSZXZpZXdlZCBieSBBZGFtIEJhcnRoLgpJbmRleDogV2ViQ29yZS9wYWdl L1hTU0F1ZGl0b3IuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGFnZS9YU1NBdWRpdG9yLmNw cAkocmV2aXNpb24gNDYyNTApCisrKyBXZWJDb3JlL3BhZ2UvWFNTQXVkaXRvci5jcHAJKHdvcmtp bmcgY29weSkKQEAgLTQ4LDYgKzQ4LDExIEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogc3RhdGlj IGJvb2wgaXNOb25DYW5vbmljYWxDaGFyYWN0ZXIoVUNoYXIgYykKIHsKKyAgICAvLyBOb3RlLCB3 ZSBkb24ndCByZW1vdmUgYmFja3NsYXNoZXMgbGlrZSBQSFAgc3RyaXBzbGFzaGVzKCksIHdoaWNo IGFtb25nIG90aGVyIHRoaW5ncyBjb252ZXJ0cyAiXFwwIiB0byB0aGUgXDAgY2hhcmFjdGVyLgor ICAgIC8vIEluc3RlYWQsIHdlIHJlbW92ZSBiYWNrc2xhc2hlcyBhbmQgemVyb3MgKHNpbmNlIHRo ZSBzdHJpbmcgIlxcMCIgPShyZW1vdmUgYmFja3NsYXNoZXMpPT4gIjAiKS4gSG93ZXZlciwgdGhp cyBoYXMgdGhlIAorICAgIC8vIGFkdmVyc2UgZWZmZWN0IHRoYXQgd2UgcmVtb3ZlIGFueSBsZWdp dGltYXRlIHplcm9zIGZyb20gYSBzdHJpbmcuCisgICAgLy8KKyAgICAvLyBGb3IgaW5zdGFuY2U6 IG5ldyBTdHJpbmcoImh0dHA6Ly9sb2NhbGhvc3Q6ODAwMCIpID0+IG5ldyBTdHJpbmcoImh0dHA6 Ly9sb2NhbGhvc3Q6OCIpLgogICAgIHJldHVybiAoYyA9PSAnXFwnIHx8IGMgPT0gJzAnIHx8IGMg PCAnICcgfHwgYyA9PSAxMjcpOwogfQogCkBAIC0yMzEsNiArMjM2LDkgQEAgYm9vbCBYU1NBdWRp dG9yOjpmaW5kSW5SZXF1ZXN0KEZyYW1lKiBmcgogICAgICAgICByZXR1cm4gZmFsc2U7CiAKICAg ICBTdHJpbmcgY2Fub25pY2FsaXplZFN0cmluZyA9IGNhbm9uaWNhbGl6ZShzdHJpbmcpOworICAg IGlmIChjYW5vbmljYWxpemVkU3RyaW5nLmlzRW1wdHkoKSkKKyAgICAgICAgcmV0dXJuIGZhbHNl OworCiAgICAgaWYgKHN0cmluZy5sZW5ndGgoKSA8IHBhZ2VVUkwubGVuZ3RoKCkpIHsKICAgICAg ICAgLy8gVGhlIHN0cmluZyBjYW4gYWN0dWFsbHkgZml0IGluc2lkZSB0aGUgcGFnZVVSTC4KICAg ICAgICAgU3RyaW5nIGRlY29kZWRQYWdlVVJMID0gY2Fub25pY2FsaXplKGRlY29kZVVSTChwYWdl VVJMLCBmcmFtZS0+ZG9jdW1lbnQoKS0+ZGVjb2RlcigpLT5lbmNvZGluZygpLCBkZWNvZGVIVE1M ZW50aXRpZXMpKTsK 33464 2009-07-24 12:52:21 -0700 2009-07-24 14:41:31 -0700 Patch with test Bug27639_2.patch text/plain 3565 dbates SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NjM3MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMDktMDctMjQgIERhbmllbCBCYXRlcyAgPGRiYXRlc0BpbnR1ZGF0 YS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI3NjM5CisKKyAgICAgICAg Rml4ZXMgZmFsc2UgcG9zaXRpdmVzIHdoZW4gZXZhbHVhdGluZyBjZXJ0YWluIHN0cmluZ3MgdGhh dCBvbmx5IGNvbnRhaW4gCisgICAgICAgIG5vbi1jYW5vbmljYWwgY2hhcmFjdGVycy4KKworICAg ICAgICBUZXN0OiBodHRwL3Rlc3RzL3NlY3VyaXR5L3hzc0F1ZGl0b3Ivc2NyaXB0LXRhZy1zYWZl Lmh0bWwKKworICAgICAgICAqIHBhZ2UvWFNTQXVkaXRvci5jcHA6CisgICAgICAgIChXZWJDb3Jl Ojppc05vbkNhbm9uaWNhbENoYXJhY3Rlcik6CisgICAgICAgIChXZWJDb3JlOjpYU1NBdWRpdG9y OjpmaW5kSW5SZXF1ZXN0KToKKwogMjAwOS0wNy0yNCAgRHJldyBXaWxzb24gIDxhdHdpbHNvbkBn b29nbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFkYW0gQmFydGguCkluZGV4OiBXZWJD b3JlL3BhZ2UvWFNTQXVkaXRvci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wYWdlL1hTU0F1 ZGl0b3IuY3BwCShyZXZpc2lvbiA0NjI1MCkKKysrIFdlYkNvcmUvcGFnZS9YU1NBdWRpdG9yLmNw cAkod29ya2luZyBjb3B5KQpAQCAtNDgsNiArNDgsMTEgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewog CiBzdGF0aWMgYm9vbCBpc05vbkNhbm9uaWNhbENoYXJhY3RlcihVQ2hhciBjKQogeworICAgIC8v IE5vdGUsIHdlIGRvbid0IHJlbW92ZSBiYWNrc2xhc2hlcyBsaWtlIFBIUCBzdHJpcHNsYXNoZXMo KSwgd2hpY2ggYW1vbmcgb3RoZXIgdGhpbmdzIGNvbnZlcnRzICJcXDAiIHRvIHRoZSBcMCBjaGFy YWN0ZXIuCisgICAgLy8gSW5zdGVhZCwgd2UgcmVtb3ZlIGJhY2tzbGFzaGVzIGFuZCB6ZXJvcyAo c2luY2UgdGhlIHN0cmluZyAiXFwwIiA9KHJlbW92ZSBiYWNrc2xhc2hlcyk9PiAiMCIpLiBIb3dl dmVyLCB0aGlzIGhhcyB0aGUgCisgICAgLy8gYWR2ZXJzZSBlZmZlY3QgdGhhdCB3ZSByZW1vdmUg YW55IGxlZ2l0aW1hdGUgemVyb3MgZnJvbSBhIHN0cmluZy4KKyAgICAvLworICAgIC8vIEZvciBp bnN0YW5jZTogbmV3IFN0cmluZygiaHR0cDovL2xvY2FsaG9zdDo4MDAwIikgPT4gbmV3IFN0cmlu ZygiaHR0cDovL2xvY2FsaG9zdDo4IikuCiAgICAgcmV0dXJuIChjID09ICdcXCcgfHwgYyA9PSAn MCcgfHwgYyA8ICcgJyB8fCBjID09IDEyNyk7CiB9CiAKQEAgLTIzMSw2ICsyMzYsOSBAQCBib29s IFhTU0F1ZGl0b3I6OmZpbmRJblJlcXVlc3QoRnJhbWUqIGZyCiAgICAgICAgIHJldHVybiBmYWxz ZTsKIAogICAgIFN0cmluZyBjYW5vbmljYWxpemVkU3RyaW5nID0gY2Fub25pY2FsaXplKHN0cmlu Zyk7CisgICAgaWYgKGNhbm9uaWNhbGl6ZWRTdHJpbmcuaXNFbXB0eSgpKQorICAgICAgICByZXR1 cm4gZmFsc2U7CisKICAgICBpZiAoc3RyaW5nLmxlbmd0aCgpIDwgcGFnZVVSTC5sZW5ndGgoKSkg ewogICAgICAgICAvLyBUaGUgc3RyaW5nIGNhbiBhY3R1YWxseSBmaXQgaW5zaWRlIHRoZSBwYWdl VVJMLgogICAgICAgICBTdHJpbmcgZGVjb2RlZFBhZ2VVUkwgPSBjYW5vbmljYWxpemUoZGVjb2Rl VVJMKHBhZ2VVUkwsIGZyYW1lLT5kb2N1bWVudCgpLT5kZWNvZGVyKCktPmVuY29kaW5nKCksIGRl Y29kZUhUTUxlbnRpdGllcykpOwpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIExheW91dFRlc3RzL0NoYW5nZUxvZwkocmV2aXNpb24gNDYzNzApCisrKyBMYXlvdXRU ZXN0cy9DaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0wNy0y NCAgRGFuaWVsIEJhdGVzICA8ZGJhdGVzQGludHVkYXRhLmNvbT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9Mjc2MzkKKyAgICAgICAgCisgICAgICAgIFRlc3RzIHRoYXQgWFNTQXVk aXRvciBkb2VzIG5vdCBwcmV2ZW50IGV2YWx1YXRpb24gb2Ygc2NyaXB0cyB0aGF0IG9ubHkgY29u dGFpbiAKKyAgICAgICAgbm9uLWNhbm9uaWNhbCBjaGFyYWN0ZXJzLgorCisgICAgICAgICogaHR0 cC90ZXN0cy9zZWN1cml0eS94c3NBdWRpdG9yL3NjcmlwdC10YWctc2FmZS1leHBlY3RlZC50eHQ6 IEFkZGVkLgorICAgICAgICAqIGh0dHAvdGVzdHMvc2VjdXJpdHkveHNzQXVkaXRvci9zY3JpcHQt dGFnLXNhZmUuaHRtbDogQWRkZWQuCisKIDIwMDktMDctMjQgIEVyaWMgU2VpZGVsICA8ZXJpY0B3 ZWJraXQub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFkYW0gQmFydGguCkluZGV4OiBMYXlv dXRUZXN0cy9odHRwL3Rlc3RzL3NlY3VyaXR5L3hzc0F1ZGl0b3Ivc2NyaXB0LXRhZy1zYWZlLWV4 cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9odHRwL3Rlc3RzL3NlY3VyaXR5 L3hzc0F1ZGl0b3Ivc2NyaXB0LXRhZy1zYWZlLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKKysr IExheW91dFRlc3RzL2h0dHAvdGVzdHMvc2VjdXJpdHkveHNzQXVkaXRvci9zY3JpcHQtdGFnLXNh ZmUtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQpAQCAtMCwwICsxIEBACisKSW5kZXg6IExheW91 dFRlc3RzL2h0dHAvdGVzdHMvc2VjdXJpdHkveHNzQXVkaXRvci9zY3JpcHQtdGFnLXNhZmUuaHRt bAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9odHRwL3Rlc3RzL3NlY3VyaXR5L3hzc0F1ZGl0 b3Ivc2NyaXB0LXRhZy1zYWZlLmh0bWwJKHJldmlzaW9uIDApCisrKyBMYXlvdXRUZXN0cy9odHRw L3Rlc3RzL3NlY3VyaXR5L3hzc0F1ZGl0b3Ivc2NyaXB0LXRhZy1zYWZlLmh0bWwJKHJldmlzaW9u IDApCkBAIC0wLDAgKzEsMTQgQEAKKzwhRE9DVFlQRSBodG1sPgorPGh0bWw+Cis8aGVhZD4KKzxz Y3JpcHQ+CitpZiAod2luZG93LmxheW91dFRlc3RDb250cm9sbGVyKSB7CisgIGxheW91dFRlc3RD b250cm9sbGVyLmR1bXBBc1RleHQoKTsKKyAgbGF5b3V0VGVzdENvbnRyb2xsZXIuc2V0WFNTQXVk aXRvckVuYWJsZWQodHJ1ZSk7Cit9Cis8L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5PgorPHNjcmlw dD4wPC9zY3JpcHQ+Cis8L2JvZHk+Cis8L2h0bWw+Cg==