[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75
dbates at webkit.org
dbates at webkit.org
Thu Oct 29 20:43:10 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit b3bfb24e7f1458eac6692cce3a9244ef1dcc205c
Author: dbates at webkit.org <dbates at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Oct 12 06:50:09 2009 +0000
2009-10-11 Daniel Bates <dbates at webkit.org>
Reviewed by Adam Barth.
https://bugs.webkit.org/show_bug.cgi?id=30242
Fixes an issue where JavaScript URLs that are URL-encoded twice can
bypass the XSSAuditor.
JavaScript URLs that are completed by method Document::completeURL have added
URL-encoded characters such that a direct comparison with the URL-decoded
outgoing HTTP parameters is not sufficient. Instead, the URL-decoded outgoing
HTTP parameters must be URL-decoded before comparison.
Tests: http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html
http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html
* bindings/ScriptControllerBase.cpp:
(WebCore::ScriptController::executeIfJavaScriptURL): Modified to pass XSSAuditor
the URL-decoded source code for the JavaScript URL.
* page/XSSAuditor.cpp:
(WebCore::isIllegalURICharacter): Minor syntactical change to the comment.
(WebCore::XSSAuditor::CachingURLCanonicalizer::canonicalizeURL): Added
parameter decodeURLEscapeSequencesTwice.
(WebCore::XSSAuditor::canEvaluateJavaScriptURL):
(WebCore::XSSAuditor::decodeURL): Ditto.
(WebCore::XSSAuditor::findInRequest): Ditto.
* page/XSSAuditor.h:
(WebCore::XSSAuditor::CachingURLCanonicalizer::CachingURLCanonicalizer): Ditto.
2009-10-11 Daniel Bates <dbates at webkit.org>
Reviewed by Adam Barth.
https://bugs.webkit.org/show_bug.cgi?id=30242
Tests that JavaScript URLs that are twice URL encoded do not bypass the XSSAuditor.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt: Added.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html: Added.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt: Added.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html: Added.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt: Added.
* http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@49434 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9d019c4..ed56d9a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2009-10-11 Daniel Bates <dbates at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ https://bugs.webkit.org/show_bug.cgi?id=30242
+
+ Tests that JavaScript URLs that are twice URL encoded do not bypass the XSSAuditor.
+
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt: Added.
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html: Added.
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt: Added.
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html: Added.
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt: Added.
+ * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html: Added.
+
2009-10-11 Dan Bernstein <mitz at apple.com>
Reviewed by Darin Adler.
diff --git a/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt
similarity index 100%
copy from LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt
copy to LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt
diff --git a/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html
new file mode 100644
index 0000000..75e595d
--- /dev/null
+++ b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src='http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<iframe src="javascript: %250Aalert(/XSS/)"></iframe>'>
+</iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt
similarity index 100%
copy from LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt
copy to LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt
diff --git a/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
new file mode 100644
index 0000000..0a3ca9f
--- /dev/null
+++ b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src='http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<iframe src="javascript: //%250Aalert(/XSS/)"></iframe>'>
+</iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt
similarity index 100%
copy from LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt
copy to LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt
diff --git a/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html
new file mode 100644
index 0000000..4687071
--- /dev/null
+++ b/LayoutTests/http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src='http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<iframe src="javascript://%250Aalert(/XSS/)"></iframe>'>
+</iframe>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 8c1f88f..6e1404f 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2009-10-11 Daniel Bates <dbates at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ https://bugs.webkit.org/show_bug.cgi?id=30242
+
+ Fixes an issue where JavaScript URLs that are URL-encoded twice can
+ bypass the XSSAuditor.
+
+ JavaScript URLs that are completed by method Document::completeURL have added
+ URL-encoded characters such that a direct comparison with the URL-decoded
+ outgoing HTTP parameters is not sufficient. Instead, the URL-decoded outgoing
+ HTTP parameters must be URL-decoded before comparison.
+
+ Tests: http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html
+ http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
+ http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html
+
+ * bindings/ScriptControllerBase.cpp:
+ (WebCore::ScriptController::executeIfJavaScriptURL): Modified to pass XSSAuditor
+ the URL-decoded source code for the JavaScript URL.
+ * page/XSSAuditor.cpp:
+ (WebCore::isIllegalURICharacter): Minor syntactical change to the comment.
+ (WebCore::XSSAuditor::CachingURLCanonicalizer::canonicalizeURL): Added
+ parameter decodeURLEscapeSequencesTwice.
+ (WebCore::XSSAuditor::canEvaluateJavaScriptURL):
+ (WebCore::XSSAuditor::decodeURL): Ditto.
+ (WebCore::XSSAuditor::findInRequest): Ditto.
+ * page/XSSAuditor.h:
+ (WebCore::XSSAuditor::CachingURLCanonicalizer::CachingURLCanonicalizer): Ditto.
+
2009-10-11 Dominic Cooney <dominicc at google.com>
Reviewed by Adam Barth.
diff --git a/WebCore/bindings/ScriptControllerBase.cpp b/WebCore/bindings/ScriptControllerBase.cpp
index 0395dec..4d73d83 100644
--- a/WebCore/bindings/ScriptControllerBase.cpp
+++ b/WebCore/bindings/ScriptControllerBase.cpp
@@ -63,10 +63,10 @@ bool ScriptController::executeIfJavaScriptURL(const KURL& url, bool userGesture,
const int javascriptSchemeLength = sizeof("javascript:") - 1;
- String script = url.string().substring(javascriptSchemeLength);
+ String script = decodeURLEscapeSequences(url.string().substring(javascriptSchemeLength));
ScriptValue result;
if (xssAuditor()->canEvaluateJavaScriptURL(script))
- result = executeScript(decodeURLEscapeSequences(script), userGesture);
+ result = executeScript(script, userGesture);
String scriptResult;
if (!result.getString(scriptResult))
diff --git a/WebCore/page/XSSAuditor.cpp b/WebCore/page/XSSAuditor.cpp
index 92ed896..3495077 100644
--- a/WebCore/page/XSSAuditor.cpp
+++ b/WebCore/page/XSSAuditor.cpp
@@ -65,20 +65,23 @@ static bool isIllegalURICharacter(UChar c)
// in a valid URI: ', ", <, >
//
// If the request does not contain these characters then we can assume that no inline scripts have been injected
- // into response page, because it is impossible to write an inline script of the form <script>...</script>
+ // into the response page, because it is impossible to write an inline script of the form <script>...</script>
// without "<", ">".
return (c == '\'' || c == '"' || c == '<' || c == '>');
}
-String XSSAuditor::CachingURLCanonicalizer::canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities)
+String XSSAuditor::CachingURLCanonicalizer::canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
+ bool decodeURLEscapeSequencesTwice)
{
- if (decodeEntities == m_decodeEntities && encoding == m_encoding && url == m_inputURL)
+ if (decodeEntities == m_decodeEntities && decodeURLEscapeSequencesTwice == m_decodeURLEscapeSequencesTwice
+ && encoding == m_encoding && url == m_inputURL)
return m_cachedCanonicalizedURL;
- m_cachedCanonicalizedURL = canonicalize(decodeURL(url, encoding, decodeEntities));
+ m_cachedCanonicalizedURL = canonicalize(decodeURL(url, encoding, decodeEntities, decodeURLEscapeSequencesTwice));
m_inputURL = url;
m_encoding = encoding;
m_decodeEntities = decodeEntities;
+ m_decodeURLEscapeSequencesTwice = decodeURLEscapeSequencesTwice;
return m_cachedCanonicalizedURL;
}
@@ -115,7 +118,7 @@ bool XSSAuditor::canEvaluateJavaScriptURL(const String& code) const
if (!isEnabled())
return true;
- if (findInRequest(code)) {
+ if (findInRequest(code, true, false, true)) {
DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
return false;
@@ -182,7 +185,7 @@ String XSSAuditor::canonicalize(const String& string)
return result.removeCharacters(&isNonCanonicalCharacter);
}
-String XSSAuditor::decodeURL(const String& string, const TextEncoding& encoding, bool decodeEntities)
+String XSSAuditor::decodeURL(const String& string, const TextEncoding& encoding, bool decodeEntities, bool decodeURLEscapeSequencesTwice)
{
String result;
String url = string;
@@ -193,6 +196,13 @@ String XSSAuditor::decodeURL(const String& string, const TextEncoding& encoding,
String decodedResult = encoding.decode(utf8Url.data(), utf8Url.length());
if (!decodedResult.isEmpty())
result = decodedResult;
+ if (decodeURLEscapeSequencesTwice) {
+ result = decodeURLEscapeSequences(result);
+ utf8Url = result.utf8();
+ decodedResult = encoding.decode(utf8Url.data(), utf8Url.length());
+ if (!decodedResult.isEmpty())
+ result = decodedResult;
+ }
if (decodeEntities)
result = decodeHTMLEntities(result);
return result;
@@ -235,18 +245,20 @@ String XSSAuditor::decodeHTMLEntities(const String& string, bool leaveUndecodabl
return String::adopt(result);
}
-bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters) const
+bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
+ bool decodeURLEscapeSequencesTwice) const
{
bool result = false;
Frame* parentFrame = m_frame->tree()->parent();
if (parentFrame && m_frame->document()->url() == blankURL())
- result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters);
+ result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
if (!result)
- result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters);
+ result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
return result;
}
-bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters) const
+bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
+ bool decodeURLEscapeSequencesTwice) const
{
ASSERT(frame->document());
@@ -285,7 +297,7 @@ bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEn
if (string.length() < pageURL.length()) {
// The string can actually fit inside the pageURL.
- String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities);
+ String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
if (allowRequestIfNoIllegalURICharacters && (!formDataObj || formDataObj->isEmpty())
&& decodedPageURL.find(&isIllegalURICharacter, 0) == -1)
@@ -302,7 +314,7 @@ bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEn
// the url-encoded POST data because the length of the url-decoded
// code is less than or equal to the length of the url-encoded
// string.
- String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities);
+ String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
if (decodedFormData.find(canonicalizedString, 0, false) != -1)
return true; // We found the string in the POST data.
}
diff --git a/WebCore/page/XSSAuditor.h b/WebCore/page/XSSAuditor.h
index d3d1ec9..adfa5c7 100644
--- a/WebCore/page/XSSAuditor.h
+++ b/WebCore/page/XSSAuditor.h
@@ -102,25 +102,30 @@ namespace WebCore {
class CachingURLCanonicalizer
{
public:
- CachingURLCanonicalizer() : m_decodeEntities(false) { }
- String canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities);
+ CachingURLCanonicalizer() : m_decodeEntities(false), m_decodeURLEscapeSequencesTwice(false) { }
+ String canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
+ bool decodeURLEscapeSequencesTwice);
private:
// The parameters we were called with last.
String m_inputURL;
TextEncoding m_encoding;
bool m_decodeEntities;
+ bool m_decodeURLEscapeSequencesTwice;
// The cached result.
String m_cachedCanonicalizedURL;
};
static String canonicalize(const String&);
- static String decodeURL(const String& url, const TextEncoding& encoding, bool decodeEntities);
+ static String decodeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
+ bool decodeURLEscapeSequencesTwice = false);
static String decodeHTMLEntities(const String&, bool leaveUndecodableEntitiesUntouched = true);
- bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false) const;
- bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false) const;
+ bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
+ bool decodeURLEscapeSequencesTwice = false) const;
+ bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
+ bool decodeURLEscapeSequencesTwice = false) const;
// The frame to audit.
Frame* m_frame;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list