Bug 111264

Summary:

XSSAuditor::eraseDangerousAttributesIfInjected shouldn't malloc for each attribute.

Product:

WebKit

Reporter:

Mike West <mkwst>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

abarth, bfulgham, dbates, dglazkov, dsavints, eric, esprehn+autocc, ojan.autocc, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

111249

Bug Blocks:

Attachments:

Description	Flags
Patch	webkit.review.bot: commit-queue-

Mike West

Reported 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. :)

Attachments
Patch (3.06 KB, patch) 2013-03-03 01:06 PST, Mike West	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Adam Barth

Comment 1 2013-03-03 01:06:44 PST

Thanks for investigating this issue.

Mike West

Comment 2 2013-03-03 01:06:50 PST

Created attachment 191130 [details] Patch

Mike West

Comment 3 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?.

WebKit Review Bot

Comment 4 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

Brent Fulgham

Comment 5 2021-09-21 14:27:50 PDT

This code is removed in Bug 230499.

Note You need to log in before you can comment on or make changes to this bug.