111264 2013-03-03 01:04:58 -0800 XSSAuditor::eraseDangerousAttributesIfInjected shouldn't malloc for each attribute. 2021-09-21 14:27:50 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 111249 1 mkwst webkit-unassigned abarth bfulgham dbates dglazkov dsavints eric esprehn+autocc ojan.autocc webkit.review.bot oldest_to_newest 845968 0 mkwst 2013-03-03 01:04:58 -0800 As noted in bug 111071 and https://bugs.webkit.org/show_bug.cgi?id=111249#c4, we're doing more work than we should in XSSAuditor::eraseDangerousAttributesIfInjected. We end up mallocing for every attribute in the document, which isn't awesome. I took a quick pass at dropping the String() call by simplifying the logic of the check, which gets 90% of the way there. Unfortunately, 'protocolIsJavaScript()' ends up doing some important work down in platform-specific code (url_util::FindAndCompareScheme) to strip out control characters that are ignored in the context of an HTML document. It's a bit ugly. I'll upload the patch I was playing with, if only as an example of how not to go about dropping this constructor. :) 845969 1 abarth 2013-03-03 01:06:44 -0800 Thanks for investigating this issue. 845970 2 191130 mkwst 2013-03-03 01:06:50 -0800 Created attachment 191130 Patch 845971 3 mkwst 2013-03-03 01:11:00 -0800 (In reply to comment #2) > Created an attachment (id=191130) [details] > Patch Er, sorry about the watchlist spam. I meant to upload this without r?. 846010 4 191130 webkit.review.bot 2013-03-03 06:54:34 -0800 Comment on attachment 191130 Patch Attachment 191130 did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16899396 New failing tests: http/tests/security/xssAuditor/iframe-injection.html http/tests/security/xssAuditor/full-block-base-href.html http/tests/security/xssAuditor/formaction-on-input.html http/tests/security/xssAuditor/embed-tag-null-char.html http/tests/security/xssAuditor/embed-tag-javascript-url.html http/tests/security/xssAuditor/cookie-injection.html http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event.html http/tests/security/xssAuditor/embed-tag-control-char.html http/tests/cache/subresource-failover-to-network.html http/tests/security/xssAuditor/full-block-javascript-link.html http/tests/security/xssAuditor/form-action.html http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event-null-char.html http/tests/security/xssAuditor/base-href-scheme-relative.html http/tests/security/xssAuditor/anchor-url-dom-write-location-javascript-URL.html http/tests/security/xssAuditor/iframe-injection-allowed.html http/tests/security/xssAuditor/cached-frame.html http/tests/security/xssAuditor/dom-write-location-javascript-URL.html http/tests/security/xssAuditor/iframe-injection-allowed-2.html http/tests/security/xssAuditor/formaction-on-button.html http/tests/security/xssAuditor/embed-tag.html http/tests/security/xssAuditor/embed-tag-code-attribute-2.html http/tests/security/xssAuditor/base-href.html http/tests/security/xssAuditor/full-block-iframe-javascript-url.html http/tests/security/xssAuditor/base-href-null-char.html http/tests/security/xssAuditor/dom-write-location-inline-event.html http/tests/security/xssAuditor/full-block-script-tag-with-source.html http/tests/security/xssAuditor/iframe-injection-allowed-3.html http/tests/security/xssAuditor/embed-tag-code-attribute.html http/tests/security/xssAuditor/base-href-control-char.html http/tests/security/xssAuditor/full-block-object-tag.html 1795770 5 bfulgham 2021-09-21 14:27:50 -0700 This code is removed in Bug 230499. 191130 2013-03-03 01:06:50 -0800 2013-03-03 06:54:34 -0800 Patch bug-111264-20130303100258.patch text/plain 3136 mkwst U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ0NTQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDE4NTA0MTE5ODI1ZjA3 YWQxYWQ2YmIxNDU1ZDQ1ZGJjMjAzYjE5ZS4uNTBiM2Q5MzEzNzU5NTM2MTMzZTczMjcwNDBjOTg1 ODQyY2ViMTEyNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEzLTAzLTAzICBNaWtl IFdlc3QgIDxta3dzdEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgWFNTQXVkaXRvcjo6ZXJhc2VE YW5nZXJvdXNBdHRyaWJ1dGVzSWZJbmplY3RlZCBzaG91bGRuJ3QgbWFsbG9jIGZvciBlYWNoIGF0 dHJpYnV0ZS4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTExMTI2NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFRoaXMgcGF0Y2ggZG9lc24ndCB3b3JrOiB1cGxvYWRpbmcgZm9yIGRpc2N1c3Npb24sIG5vdCBm b3IgbGFuZGluZyAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cyAoT09QUyEpLgorCisg ICAgICAgICogaHRtbC9wYXJzZXIvWFNTQXVkaXRvci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpo YXNKYXZhU2NyaXB0UHJvdG9jb2wpOgorICAgICAgICAoV2ViQ29yZSk6CisgICAgICAgIChXZWJD b3JlOjpYU1NBdWRpdG9yOjplcmFzZURhbmdlcm91c0F0dHJpYnV0ZXNJZkluamVjdGVkKToKKwog MjAxMy0wMy0wMiAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBG aXggYSB0eXBvIGluIG15IHByZXZpb3VzIGNvbW1pdCAocjE0NDUzNCkuCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9YU1NBdWRpdG9yLmNwcCBiL1NvdXJjZS9XZWJDb3Jl L2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3IuY3BwCmluZGV4IDNhYTY1YTcxMzZjM2YyN2I1YTM2MDgy NmNhMzczNzRiYjdiN2QyNTcuLmM1MzRlMTU4OTY5ZTg1NWFjYzRkNTcyZmYyOGRkMWY1OWEwZDVk MDQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3IuY3Bw CisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3IuY3BwCkBAIC00OTcs NiArNDk3LDI5IEBAIGJvb2wgWFNTQXVkaXRvcjo6ZmlsdGVyQnV0dG9uVG9rZW4oY29uc3QgRmls dGVyVG9rZW5SZXF1ZXN0JiByZXF1ZXN0KQogICAgIHJldHVybiBlcmFzZUF0dHJpYnV0ZUlmSW5q ZWN0ZWQocmVxdWVzdCwgZm9ybWFjdGlvbkF0dHIsIGJsYW5rVVJMKCkuc3RyaW5nKCksIFNyY0xp a2VBdHRyaWJ1dGUpOwogfQogCitzdGF0aWMgYm9vbCBoYXNKYXZhU2NyaXB0UHJvdG9jb2woY29u c3QgVmVjdG9yPFVDaGFyLCAzMj4mIHZhbHVlKQoreworICAgIHByaW50ZigiVmFsdWU6ICclcydc biIsIFN0cmluZyh2YWx1ZSkudXRmOCgpLmRhdGEoKSk7CisgICAgc2l6ZV90IGxlbmd0aE9mSmF2 YVNjcmlwdFByb3RvY29sID0gMTE7CisgICAgaWYgKHZhbHVlLnNpemUoKSA8IGxlbmd0aE9mSmF2 YVNjcmlwdFByb3RvY29sKQorICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICBzaXplX3QgY3Vy cmVudCA9IDA7CisgICAgZm9yICg7IGN1cnJlbnQgPCB2YWx1ZS5zaXplKCk7ICsrY3VycmVudCkg eworICAgICAgICBwcmludGYoIkNoZWNraW5nOiAnJWMnXG4iLCB2YWx1ZVtjdXJyZW50XSk7Cisg ICAgICAgIGlmIChpc05vdEhUTUxTcGFjZSh2YWx1ZVtjdXJyZW50XSkpCisgICAgICAgICAgICBi cmVhazsKKyAgICB9CisKKyAgICBpZiAoY3VycmVudCA9PSB2YWx1ZS5zaXplKCkgfHwgdmFsdWUu c2l6ZSgpIC0gY3VycmVudCA8IGxlbmd0aE9mSmF2YVNjcmlwdFByb3RvY29sKQorICAgICAgICBy ZXR1cm4gZmFsc2U7CisKKyAgICAvLyBfX19fcHJvdG9jb2w6Li4uCisgICAgLy8gICAgIF4KKyAg ICBjb25zdCBVQ2hhciogc3RhcnRPZlByb3RvY29sID0gdmFsdWUuZGF0YSgpICsgKHNpemVvZihV Q2hhcikgKiBjdXJyZW50KTsKKyAgICByZXR1cm4gZXF1YWxJZ25vcmluZ0Nhc2Uoc3RhcnRPZlBy b3RvY29sLCAiamF2YXNjcmlwdDoiLCBsZW5ndGhPZkphdmFTY3JpcHRQcm90b2NvbCk7Cit9CisK IGJvb2wgWFNTQXVkaXRvcjo6ZXJhc2VEYW5nZXJvdXNBdHRyaWJ1dGVzSWZJbmplY3RlZChjb25z dCBGaWx0ZXJUb2tlblJlcXVlc3QmIHJlcXVlc3QpCiB7CiAgICAgREVGSU5FX1NUQVRJQ19MT0NB TChTdHJpbmcsIHNhZmVKYXZhU2NyaXB0VVJMLCAoQVNDSUlMaXRlcmFsKCJqYXZhc2NyaXB0OnZv aWQoMCkiKSkpOwpAQCAtNTA1LDcgKzUyOCw3IEBAIGJvb2wgWFNTQXVkaXRvcjo6ZXJhc2VEYW5n ZXJvdXNBdHRyaWJ1dGVzSWZJbmplY3RlZChjb25zdCBGaWx0ZXJUb2tlblJlcXVlc3QmIHJlCiAg ICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCByZXF1ZXN0LnRva2VuLmF0dHJpYnV0ZXMoKS5zaXpl KCk7ICsraSkgewogICAgICAgICBjb25zdCBIVE1MVG9rZW46OkF0dHJpYnV0ZSYgYXR0cmlidXRl ID0gcmVxdWVzdC50b2tlbi5hdHRyaWJ1dGVzKCkuYXQoaSk7CiAgICAgICAgIGJvb2wgaXNJbmxp bmVFdmVudEhhbmRsZXIgPSBpc05hbWVPZklubGluZUV2ZW50SGFuZGxlcihhdHRyaWJ1dGUubmFt ZSk7Ci0gICAgICAgIGJvb2wgdmFsdWVDb250YWluc0phdmFTY3JpcHRVUkwgPSAhaXNJbmxpbmVF dmVudEhhbmRsZXIgJiYgcHJvdG9jb2xJc0phdmFTY3JpcHQoc3RyaXBMZWFkaW5nQW5kVHJhaWxp bmdIVE1MU3BhY2VzKFN0cmluZyhhdHRyaWJ1dGUudmFsdWUpKSk7CisgICAgICAgIGJvb2wgdmFs dWVDb250YWluc0phdmFTY3JpcHRVUkwgPSAhaXNJbmxpbmVFdmVudEhhbmRsZXIgJiYgaGFzSmF2 YVNjcmlwdFByb3RvY29sKGF0dHJpYnV0ZS52YWx1ZSk7CiAgICAgICAgIGlmICghaXNJbmxpbmVF dmVudEhhbmRsZXIgJiYgIXZhbHVlQ29udGFpbnNKYXZhU2NyaXB0VVJMKQogICAgICAgICAgICAg Y29udGludWU7CiAgICAgICAgIGlmICghaXNDb250YWluZWRJblJlcXVlc3QoZGVjb2RlZFNuaXBw ZXRGb3JBdHRyaWJ1dGUocmVxdWVzdCwgYXR0cmlidXRlLCBTY3JpcHRMaWtlQXR0cmlidXRlKSkp Cg==