[webkit-reviews] review requested: [Bug 20651] svgElement.className.baseValue does not work : [Attachment 24168] WebCore:

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has asked  for review:
Bug 20651: svgElement.className.baseValue does not work
https://bugs.webkit.org/show_bug.cgi?id=20651

Attachment 24168: WebCore:
https://bugs.webkit.org/attachment.cgi?id=24168&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>

	Reviewed by Sam Weinig

	Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523
	<rdar://problem/5657447>

	When a frame is created with the URL "about:blank" or "", it should
	inherit its SecurityOrigin from its opener.  However, once it has
	decided on that SecurityOrigin, it should not change its mind.
	Prior to this patch, several events could induce the frame to change
	its SecurityOrigin, permitting an attacker to inject script into an
	arbitrary SecurityOrigin.

	This patch makes several changes:

	1) Documents refuse to change from one SecurityOrigin to another
	   unless explicitly instructed to do so.

	2) Navigating to a JavaScript URL that produces a value
	   preserves the current SecurityOrigin explicitly instead of
	   relying on the URL to preserve the origin (which fails for
	   about:blank URLs and SecurityOrigins with document.domain set).

	   Ideally, we should not preserve the URL at all.  Instead, the
	   frame's URL should be the JavaScript URL, as in Firefox, but this
	   would require changes that are too risky for this patch.  I'll
	   file this as a separate issue.

	3) Various methods of navigating to JavaScript URLs were not
	   properly handling JavaScript that returned a value (and should
	   therefore replace the current document).  This patch unifies
	   those code paths with the path that works.

	   There are still a handful of bugs relating to the handling of
	   JavaScript URLs, but I'll file those as separate issues.

	Tests:
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html
	      
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
	       http/tests/security/aboutBlank/xss-DENIED-set-opener.html

	* dom/Document.cpp:
	(WebCore::Document::initSecurityOrigin):
	* dom/Document.h:
	(WebCore::Document::setSecurityOrigin):
	* loader/FrameLoader.cpp:
	(WebCore::FrameLoader::changeLocation):
	(WebCore::FrameLoader::urlSelected):
	(WebCore::FrameLoader::requestFrame):
	(WebCore::FrameLoader::submitForm):
	(WebCore::FrameLoader::executeIfJavaScriptURL):
	(WebCore::FrameLoader::begin):
	* loader/FrameLoader.h:
	* platform/SecurityOrigin.cpp:
	(WebCore::SecurityOrigin::setForURL):
	(WebCore::SecurityOrigin::createForFrame):
	* platform/SecurityOrigin.h:

LayoutTests:

	Reviewed by Sam Weinig.

	Fixes: http://bugs.webkit.org/show_bug.cgi?id=16523

	Adds new LayoutTests for scripting from about:blank windows.  These
	windows should inherit its SecurityOrigin from its opener and should
	refuse to change their origins when their opener changes exogenously
	(the navigate-opener tests) or explicitly (the set-opener test).

	* http/tests/security/aboutBlank: Added.
	*
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write-expect
ed.txt: Added.
	*
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html:
Added.
	*
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expect
ed.txt: Added.
	*
http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html:
Added.
	* http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt:
Added.
	* http/tests/security/aboutBlank/xss-DENIED-set-opener.html: Added.
	* http/tests/security/resources/innocent-victim-with-notify.html:
Added.
	* http/tests/security/resources/innocent-victim.html: Added.
	* http/tests/security/resources/libwrapjs.js: Added.
	* http/tests/security/resources/open-window.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@29266
268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 LayoutTests/ChangeLog				    |	23 ++++
 ...IED-navigate-opener-document-write-expected.txt |	17 +++
 .../xss-DENIED-navigate-opener-document-write.html |  105 +++++++++++++++++++
 ...IED-navigate-opener-javascript-url-expected.txt |	17 +++
 .../xss-DENIED-navigate-opener-javascript-url.html |  106 ++++++++++++++++++++
 .../aboutBlank/xss-DENIED-set-opener-expected.txt  |	20 ++++
 .../security/aboutBlank/xss-DENIED-set-opener.html |	76 ++++++++++++++
 .../resources/innocent-victim-with-notify.html     |	14 +++
 .../tests/security/resources/innocent-victim.html  |	 5 +
 .../http/tests/security/resources/libwrapjs.js     |	62 ++++++++++++
 .../http/tests/security/resources/open-window.html |	22 ++++
 LayoutTests/platform/win/Skipped		    |	 3 +
 WebCore/ChangeLog				    |	58 +++++++++++
 WebCore/dom/Document.cpp			    |	 3 +
 WebCore/dom/Document.h 			    |	 5 +
 WebCore/loader/FrameLoader.cpp 		    |	53 +++++-----
 WebCore/loader/FrameLoader.h			    |	 8 +-
 WebCore/platform/SecurityOrigin.cpp		    |	55 ++++------
 WebCore/platform/SecurityOrigin.h		    |	 7 +-
 19 files changed, 592 insertions(+), 67 deletions(-)