Diff
Modified: trunk/LayoutTests/ChangeLog (133965 => 133966)
--- trunk/LayoutTests/ChangeLog 2012-11-08 23:50:12 UTC (rev 133965)
+++ trunk/LayoutTests/ChangeLog 2012-11-09 00:05:02 UTC (rev 133966)
@@ -1,3 +1,30 @@
+2012-11-08 Christophe Dumez <christophe.du...@intel.com>
+
+ [JSC] HTML extensions to String.prototype should escape " as " in argument values
+ https://bugs.webkit.org/show_bug.cgi?id=90667
+
+ Reviewed by Benjamin Poulain.
+
+ Add layout test coverage for the following String.prototype methods:
+ - String.prototype.anchor(name)
+ - String.prototype.fontcolor(color)
+ - String.prototype.fontsize(size)
+ - String.prototype.link(href)
+
+ Those tests also check that the quotation mark is correctly escaped
+ in argument values to prevent potential exploits.
+
+ * fast/js/string-anchor-expected.txt: Added.
+ * fast/js/string-anchor.html: Added.
+ * fast/js/string-fontcolor-expected.txt: Added.
+ * fast/js/string-fontcolor.html: Added.
+ * fast/js/string-fontsize-expected.txt: Added.
+ * fast/js/string-fontsize.html: Added.
+ * fast/js/string-link-expected.txt: Added.
+ * fast/js/string-link.html: Added.
+ * platform/chromium/TestExpectations: Skip new tests for chromium port due
+ to http://code.google.com/p/v8/issues/detail?id=2218
+
2012-11-08 Joshua Bell <jsb...@chromium.org>
[Chromium] Unreviewed gardening. Added missing *-expected.txt following rebaselines.
Added: trunk/LayoutTests/fast/js/string-anchor-expected.txt (0 => 133966)
--- trunk/LayoutTests/fast/js/string-anchor-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/string-anchor-expected.txt 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,18 @@
+This is a test case for String.prototype.anchor(name).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS '_'.anchor('b') is "<a name=\"b\">_</a>"
+PASS '<'.anchor('b') is "<a name=\"b\"><</a>"
+PASS '_'.anchor(0x2A) is "<a name=\"42\">_</a>"
+PASS '_'.anchor('"') is "<a name=\""\">_</a>"
+PASS '_'.anchor('" href="" is "<a name=\"" href=""
+PASS String.prototype.anchor.call(0x2A, 0x2A) is "<a name=\"42\">42</a>"
+PASS String.prototype.anchor.call(undefined) threw exception TypeError: Type error.
+PASS String.prototype.anchor.call(null) threw exception TypeError: Type error.
+PASS String.prototype.anchor.length is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/string-anchor.html (0 => 133966)
--- trunk/LayoutTests/fast/js/string-anchor.html (rev 0)
+++ trunk/LayoutTests/fast/js/string-anchor.html 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description(
+'This is a test case for String.prototype.anchor(name).'
+);
+
+// This test is based on http://mathias.html5.org/tests/_javascript_/string/.
+
+// Simple case.
+shouldBe("'_'.anchor('b')", '"<a name=\\"b\\">_</a>"');
+
+// Does not escape special characters in `this` value.
+shouldBe("'<'.anchor('b')", '"<a name=\\"b\\"><</a>"');
+
+// first argument gets ToString()ed.
+shouldBe("'_'.anchor(0x2A)", '"<a name=\\"42\\">_</a>"');
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.anchor('\"')", '"<a name=\\""\\">_</a>"');
+shouldBe("'_'.anchor('\" href="" '"<a name=\\"" href=""
+
+// Generic use on Number object.
+shouldBe("String.prototype.anchor.call(0x2A, 0x2A)", '"<a name=\\"42\\">42</a>"');
+
+// Generic use on non-coercible object `undefined`.
+shouldThrow("String.prototype.anchor.call(undefined)", '"TypeError: Type error"');
+
+// Generic use on non-coercible object `null`.
+shouldThrow("String.prototype.anchor.call(null)", '"TypeError: Type error"');
+
+// Check anchor.length.
+shouldBe("String.prototype.anchor.length", "1");
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/string-fontcolor-expected.txt (0 => 133966)
--- trunk/LayoutTests/fast/js/string-fontcolor-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/string-fontcolor-expected.txt 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,18 @@
+This is a test case for String.prototype.fontcolor(color).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS '_'.fontcolor('b') is "<font color=\"b\">_</font>"
+PASS '<'.fontcolor('b') is "<font color=\"b\"><</font>"
+PASS '_'.fontcolor(0x2A) is "<font color=\"42\">_</font>"
+PASS '_'.fontcolor('"') is "<font color=\""\">_</font>"
+PASS '_'.fontcolor('" size="2px') is "<font color=\"" size="2px\">_</font>"
+PASS String.prototype.fontcolor.call(0x2A, 0x2A) is "<font color=\"42\">42</font>"
+PASS String.prototype.fontcolor.call(undefined) threw exception TypeError: Type error.
+PASS String.prototype.fontcolor.call(null) threw exception TypeError: Type error.
+PASS String.prototype.fontcolor.length is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/string-fontcolor.html (0 => 133966)
--- trunk/LayoutTests/fast/js/string-fontcolor.html (rev 0)
+++ trunk/LayoutTests/fast/js/string-fontcolor.html 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,41 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description(
+'This is a test case for String.prototype.fontcolor(color).'
+);
+
+// This test is based on http://mathias.html5.org/tests/_javascript_/string/.
+
+// Simple case.
+shouldBe("'_'.fontcolor('b')", '"<font color=\\"b\\">_</font>"');
+
+// Does not escape special characters in `this` value.
+shouldBe("'<'.fontcolor('b')", '"<font color=\\"b\\"><</font>"');
+
+// First argument gets ToString()ed.
+shouldBe("'_'.fontcolor(0x2A)", '"<font color=\\"42\\">_</font>"');
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.fontcolor('\"')", '"<font color=\\""\\">_</font>"');
+shouldBe("'_'.fontcolor('\" size=\"2px')", '"<font color=\\"" size="2px\\">_</font>"');
+
+// Generic use on Number object.
+shouldBe("String.prototype.fontcolor.call(0x2A, 0x2A)", '"<font color=\\"42\\">42</font>"');
+
+// Generic use on non-coercible object `undefined`.
+shouldThrow("String.prototype.fontcolor.call(undefined)", '"TypeError: Type error"');
+
+// Generic use on non-coercible object `null`.
+shouldThrow("String.prototype.fontcolor.call(null)", '"TypeError: Type error"');
+
+// Check fontcolor.length.
+shouldBe("String.prototype.fontcolor.length", "1");
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/string-fontsize-expected.txt (0 => 133966)
--- trunk/LayoutTests/fast/js/string-fontsize-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/string-fontsize-expected.txt 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,19 @@
+This is a test case for String.prototype.fontsize(size).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS '_'.fontsize('"') is "<font size=\""\">_</font>"
+PASS '_'.fontsize('b') is "<font size=\"b\">_</font>"
+PASS '<'.fontsize('b') is "<font size=\"b\"><</font>"
+PASS '_'.fontsize(0x2A) is "<font size=\"42\">_</font>"
+PASS '_'.fontsize('"') is "<font size=\""\">_</font>"
+PASS '_'.fontsize('" color="b') is "<font size=\"" color="b\">_</font>"
+PASS String.prototype.fontsize.call(0x2A, 0x2A) is "<font size=\"42\">42</font>"
+PASS String.prototype.fontsize.call(undefined) threw exception TypeError: Type error.
+PASS String.prototype.fontsize.call(null) threw exception TypeError: Type error.
+PASS String.prototype.fontsize.length is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/string-fontsize.html (0 => 133966)
--- trunk/LayoutTests/fast/js/string-fontsize.html (rev 0)
+++ trunk/LayoutTests/fast/js/string-fontsize.html 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,44 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description(
+'This is a test case for String.prototype.fontsize(size).'
+);
+
+// This test is based on http://mathias.html5.org/tests/_javascript_/string/.
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.fontsize('\"')", '"<font size=\\""\\">_</font>"');
+
+// Simple case.
+shouldBe("'_'.fontsize('b')", '"<font size=\\"b\\">_</font>"');
+
+// Does not escape special characters in `this` value.
+shouldBe("'<'.fontsize('b')", '"<font size=\\"b\\"><</font>"');
+
+// First argument gets ToString()ed.
+shouldBe("'_'.fontsize(0x2A)", '"<font size=\\"42\\">_</font>"');
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.fontsize('\"')", '"<font size=\\""\\">_</font>"');
+shouldBe("'_'.fontsize('\" color=\"b')", '"<font size=\\"" color="b\\">_</font>"');
+
+// Generic use on Number object.
+shouldBe("String.prototype.fontsize.call(0x2A, 0x2A)", '"<font size=\\"42\\">42</font>"');
+
+// Generic use on non-coercible object `undefined`.
+shouldThrow("String.prototype.fontsize.call(undefined)", '"TypeError: Type error"');
+
+// Generic use on non-coercible object `null`.
+shouldThrow("String.prototype.fontsize.call(null)", '"TypeError: Type error"');
+
+// Check fontsize.length.
+shouldBe("String.prototype.fontsize.length", "1");
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/string-link-expected.txt (0 => 133966)
--- trunk/LayoutTests/fast/js/string-link-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/string-link-expected.txt 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,19 @@
+This is a test case for String.prototype.link(href).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS '_'.link('"') is "<a href=""
+PASS '_'.link('b') is "<a href=""
+PASS '<'.link('b') is "<a href=""
+PASS '_'.link(0x2A) is "<a href=""
+PASS '_'.link('"') is "<a href=""
+PASS '_'.link('" target="_blank') is "<a href="" target="_blank\">_</a>"
+PASS String.prototype.link.call(0x2A, 0x2A) is "<a href=""
+PASS String.prototype.link.call(undefined) threw exception TypeError: Type error.
+PASS String.prototype.link.call(null) threw exception TypeError: Type error.
+PASS String.prototype.link.length is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/string-link.html (0 => 133966)
--- trunk/LayoutTests/fast/js/string-link.html (rev 0)
+++ trunk/LayoutTests/fast/js/string-link.html 2012-11-09 00:05:02 UTC (rev 133966)
@@ -0,0 +1,44 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description(
+'This is a test case for String.prototype.link(href).'
+);
+
+// This test is based on http://mathias.html5.org/tests/_javascript_/string/.
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.link('\"')", '"<a href=""
+
+// Simple case.
+shouldBe("'_'.link('b')", '"<a href=""
+
+// Does not escape special characters in `this` value.
+shouldBe("'<'.link('b')", '"<a href=""
+
+// First argument gets ToString()ed.
+shouldBe("'_'.link(0x2A)", '"<a href=""
+
+// Check that the quotation mark is correctly escaped.
+shouldBe("'_'.link('\"')", '"<a href=""
+shouldBe("'_'.link('\" target=\"_blank')", '"<a href="" target="_blank\\">_</a>"');
+
+// Generic use on Number object.
+shouldBe("String.prototype.link.call(0x2A, 0x2A)", '"<a href=""
+
+// Generic use on non-coercible object `undefined`.
+shouldThrow("String.prototype.link.call(undefined)", '"TypeError: Type error"');
+
+// Generic use on non-coercible object `null`.
+shouldThrow("String.prototype.link.call(null)", '"TypeError: Type error"');
+
+// Check link.length.
+shouldBe("String.prototype.link.length", "1");
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (133965 => 133966)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2012-11-08 23:50:12 UTC (rev 133965)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2012-11-09 00:05:02 UTC (rev 133966)
@@ -2457,6 +2457,13 @@
# FIXME: Need to add tooling support for V8 bugs.
code.google.com/p/v8/issues/detail?id=953 fast/regex/pcre-test-1.html [ Timeout ]
+# HTML extensions to String.prototype should throw an error if the `this` object
+# is not coercible (i.e. `null` or `undefined`)
+code.google.com/p/v8/issues/detail?id=2218 fast/js/string-anchor.html [ Failure ]
+code.google.com/p/v8/issues/detail?id=2218 fast/js/string-fontcolor.html [ Failure ]
+code.google.com/p/v8/issues/detail?id=2218 fast/js/string-fontsize.html [ Failure ]
+code.google.com/p/v8/issues/detail?id=2218 fast/js/string-link.html [ Failure ]
+
webkit.org/b/50282 [ Android Linux Win ] fast/images/imagemap-focus-ring-outline-color-explicitly-inherited-from-map.html [ Failure ]
webkit.org/b/50282 [ Android Linux Win ] fast/images/imagemap-focus-ring-outline-color-not-inherited-from-map.html [ Failure ]
webkit.org/b/50282 [ Android Linux Win ] fast/images/imagemap-focus-ring-outline-color.html [ Failure ]
Modified: trunk/Source/_javascript_Core/ChangeLog (133965 => 133966)
--- trunk/Source/_javascript_Core/ChangeLog 2012-11-08 23:50:12 UTC (rev 133965)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-11-09 00:05:02 UTC (rev 133966)
@@ -1,3 +1,28 @@
+2012-11-08 Christophe Dumez <christophe.du...@intel.com>
+
+ [JSC] HTML extensions to String.prototype should escape " as " in argument values
+ https://bugs.webkit.org/show_bug.cgi?id=90667
+
+ Reviewed by Benjamin Poulain.
+
+ Escape quotation mark as " in argument values to:
+ - String.prototype.anchor(name)
+ - String.prototype.fontcolor(color)
+ - String.prototype.fontsize(size)
+ - String.prototype.link(href)
+
+ This behavior matches Chromium/V8 and Firefox/Spidermonkey
+ implementations and is requited by:
+ http://mathias.html5.org/specs/_javascript_/#escapeattributevalue
+
+ This also fixes a potential security risk (XSS vector).
+
+ * runtime/StringPrototype.cpp:
+ (JSC::stringProtoFuncFontcolor):
+ (JSC::stringProtoFuncFontsize):
+ (JSC::stringProtoFuncAnchor):
+ (JSC::stringProtoFuncLink):
+
2012-11-08 Anders Carlsson <ander...@apple.com>
HeapStatistics::s_pauseTimeStarts and s_pauseTimeEnds should be Vectors
Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (133965 => 133966)
--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2012-11-08 23:50:12 UTC (rev 133965)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2012-11-09 00:05:02 UTC (rev 133966)
@@ -1388,7 +1388,10 @@
return throwVMTypeError(exec);
String s = thisValue.toString(exec)->value(exec);
JSValue a0 = exec->argument(0);
- return JSValue::encode(jsMakeNontrivialString(exec, "<font color=\"", a0.toString(exec)->value(exec), "\">", s, "</font>"));
+ String color = a0.toWTFString(exec);
+ color.replaceWithLiteral('"', """);
+
+ return JSValue::encode(jsMakeNontrivialString(exec, "<font color=\"", color, "\">", s, "</font>"));
}
EncodedJSValue JSC_HOST_CALL stringProtoFuncFontsize(ExecState* exec)
@@ -1433,7 +1436,10 @@
return JSValue::encode(jsNontrivialString(exec, impl));
}
- return JSValue::encode(jsMakeNontrivialString(exec, "<font size=\"", a0.toString(exec)->value(exec), "\">", s, "</font>"));
+ String fontSize = a0.toWTFString(exec);
+ fontSize.replaceWithLiteral('"', """);
+
+ return JSValue::encode(jsMakeNontrivialString(exec, "<font size=\"", fontSize, "\">", s, "</font>"));
}
EncodedJSValue JSC_HOST_CALL stringProtoFuncAnchor(ExecState* exec)
@@ -1443,7 +1449,10 @@
return throwVMTypeError(exec);
String s = thisValue.toString(exec)->value(exec);
JSValue a0 = exec->argument(0);
- return JSValue::encode(jsMakeNontrivialString(exec, "<a name=\"", a0.toString(exec)->value(exec), "\">", s, "</a>"));
+ String anchor = a0.toWTFString(exec);
+ anchor.replaceWithLiteral('"', """);
+
+ return JSValue::encode(jsMakeNontrivialString(exec, "<a name=\"", anchor, "\">", s, "</a>"));
}
EncodedJSValue JSC_HOST_CALL stringProtoFuncLink(ExecState* exec)
@@ -1453,7 +1462,8 @@
return throwVMTypeError(exec);
String s = thisValue.toString(exec)->value(exec);
JSValue a0 = exec->argument(0);
- String linkText = a0.toString(exec)->value(exec);
+ String linkText = a0.toWTFString(exec);
+ linkText.replaceWithLiteral('"', """);
unsigned linkTextSize = linkText.length();
unsigned stringSize = s.length();
