Bug 27071 - [XSSAuditor] HTTP parameters with null/control characters bypass XSSAuditor
Summary: [XSSAuditor] HTTP parameters with null/control characters bypass XSSAuditor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-07 23:54 PDT by Daniel Bates
Modified: 2009-07-08 14:27 PDT (History)
3 users (show)

See Also:


Attachments
Patch with tests (29.70 KB, patch)
2009-07-08 00:02 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch with tests (29.68 KB, patch)
2009-07-08 00:12 PDT, Daniel Bates
abarth: review-
Details | Formatted Diff | Diff
Patch with tests (29.26 KB, patch)
2009-07-08 11:18 PDT, Daniel Bates
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Daniel Bates 2009-07-08 00:02:08 PDT
Created attachment 32431 [details]
Patch with tests
Comment 2 Daniel Bates 2009-07-08 00:08:22 PDT
I patched this by telling XSSAuditor::findInRequest when to allow/disallow null and non-null control characters.

I also changed the console message type in method XSSAuditor::canLoadObject from OtherMessageSource to JSMessageSource, since DumpRenderTree doesn't seem to dump n OtherMessageSource errors as needed by various plugin-based test cases.

(In reply to comment #1)
> Created an attachment (id=32431) [details]
> Patch with tests
Comment 3 Daniel Bates 2009-07-08 00:12:22 PDT
Created attachment 32433 [details]
Patch with tests

Forgot to put email address in change log for LayoutTests.
Comment 4 Adam Barth 2009-07-08 10:39:52 PDT
Comment on attachment 32433 [details]
Patch with tests

What is execGetURL.swf ?  I don't think we can put flash movies in layout tests.  This probably isn't needed because the auditor blocks the load anyway.  Also, where is script-tag-post-control-char.html ?
Comment 5 Daniel Bates 2009-07-08 11:02:18 PDT
Right. I just used it as a place holder for the plugin-based tests, but it isn't needed as you pointed out. I'll add such a test and post the patch again.

(In reply to comment #4)
> (From update of attachment 32433 [details])
> What is execGetURL.swf ?  I don't think we can put flash movies in layout
> tests.  This probably isn't needed because the auditor blocks the load anyway. 
> Also, where is script-tag-post-control-char.html ?
Comment 6 Adam Barth 2009-07-08 11:06:46 PDT
Awesome.  Thanks Dan.
Comment 7 Daniel Bates 2009-07-08 11:18:27 PDT
Created attachment 32462 [details]
Patch with tests

Removed file execGetURL.swf. Added test case script-tag-post-control-char.html
Comment 8 Adam Barth 2009-07-08 12:02:30 PDT
Comment on attachment 32462 [details]
Patch with tests

This looks great.  Thanks Dan.  The ChangeLog still lists the SWF, but that's not a big deal.  I'll try to remember to remove it when I land the patch.
Comment 9 Adam Barth 2009-07-08 14:27:24 PDT
Sending        LayoutTests/ChangeLog
Adding         LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-inline-event-null-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag-control-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag-control-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag-null-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/embed-tag.html
Adding         LayoutTests/http/tests/security/xssAuditor/link-onclick-control-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/link-onclick-control-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/link-onclick-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/link-onclick-null-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag-control-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag-control-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag-null-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/object-embed-tag.html
Adding         LayoutTests/http/tests/security/xssAuditor/object-tag-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/object-tag.html
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-post-control-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-post-control-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-post-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-post-null-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-control-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-control-char.html
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-null-char-expected.txt
Adding         LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-null-char.html
Sending        WebCore/ChangeLog
Sending        WebCore/page/XSSAuditor.cpp
Sending        WebCore/page/XSSAuditor.h
Transmitting file data ................................
Committed revision 45639.
http://trac.webkit.org/changeset/45639