Bug 111264 - XSSAuditor::eraseDangerousAttributesIfInjected shouldn't malloc for each attribute.
Summary: XSSAuditor::eraseDangerousAttributesIfInjected shouldn't malloc for each attr...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 111249
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-03 01:04 PST by Mike West
Modified: 2013-04-17 01:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.06 KB, patch)
2013-03-03 01:06 PST, Mike West
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike West 2013-03-03 01:04:58 PST
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. :)
Comment 1 Adam Barth 2013-03-03 01:06:44 PST
Thanks for investigating this issue.
Comment 2 Mike West 2013-03-03 01:06:50 PST
Created attachment 191130 [details]
Patch
Comment 3 Mike West 2013-03-03 01:11:00 PST
(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?.
Comment 4 WebKit Review Bot 2013-03-03 06:54:34 PST
Comment on attachment 191130 [details]
Patch

Attachment 191130 [details] 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