Diff
Modified: releases/WebKitGTK/webkit-2.4/LayoutTests/ChangeLog (167234 => 167235)
--- releases/WebKitGTK/webkit-2.4/LayoutTests/ChangeLog 2014-04-14 10:38:38 UTC (rev 167234)
+++ releases/WebKitGTK/webkit-2.4/LayoutTests/ChangeLog 2014-04-14 10:47:30 UTC (rev 167235)
@@ -1,3 +1,15 @@
+2014-03-18 Antti Koivisto <[email protected]>
+
+ Mutating rules returned by getMatchedCSSRules can result in crash
+ https://bugs.webkit.org/show_bug.cgi?id=130209
+
+ Reviewed by Andreas Kling.
+
+ * fast/css/getMatchedCSSProperties-rule-mutation-expected.txt: Added.
+ * fast/css/getMatchedCSSProperties-rule-mutation.html: Added.
+ * fast/css/getMatchedCSSRules-crash-expected.txt: Added.
+ * fast/css/getMatchedCSSRules-crash.html: Added.
+
2014-03-08 Oliver Hunt <[email protected]>
SerializedScriptValue may move Identifiers between worlds
Added: releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation-expected.txt (0 => 167235)
--- releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation-expected.txt 2014-04-14 10:47:30 UTC (rev 167235)
@@ -0,0 +1,20 @@
+Test that CSSStyleRules returned by getMatchedCSSRules can't be mutated
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS rules.length is 2
+PASS rules[0].selectorText is originalText
+PASS rules[1].selectorText is originalText
+PASS rules[0].style.cssText is originalText
+PASS rules[1].style.cssText is originalText
+PASS rules[0].style.color is originalText
+PASS rules[1].style.color is originalText
+PASS rules[0].style.getPropertyValue('color') is originalText
+PASS rules[1].style.getPropertyValue('color') is originalText
+PASS rules[0].style.color is originalText
+PASS rules[1].style.color is originalText
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation.html (0 => 167235)
--- releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation.html (rev 0)
+++ releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSProperties-rule-mutation.html 2014-04-14 10:47:30 UTC (rev 167235)
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+#test { color: blue; }
+@media all {
+ #test { color: blue; }
+}
+</style>
+</head>
+<body>
+<div id=test>
+</div>
+<script>
+
+description("Test that CSSStyleRules returned by getMatchedCSSRules can't be mutated");
+
+var testDiv = document.getElementById('test');
+var rules = getMatchedCSSRules(testDiv);
+
+function tryMutateSelector(index) {
+ originalText = rules[index].selectorText;
+ rules[index].selectorText = "#mutated";
+ shouldBe("rules["+index+"].selectorText", "originalText");
+}
+
+function tryMutateCSSText(index) {
+ originalText = rules[index].style.cssText;
+ rules[index].style.cssText = "color: red";
+ shouldBe("rules["+index+"].style.cssText", "originalText");
+}
+
+function tryMutateProperty(index) {
+ originalText = rules[index].style.color;
+ rules[index].style.color = "green";
+ shouldBe("rules["+index+"].style.color", "originalText");
+}
+
+function tryMutateProperty2(index) {
+ originalText = rules[index].style.getPropertyValue("color");
+ rules[index].style.setProperty("color", "white");
+ shouldBe("rules["+index+"].style.getPropertyValue('color')", "originalText");
+}
+
+function tryRemoveProperty(index) {
+ originalText = rules[index].style.color;
+ rules[index].style.removeProperty("color");
+ shouldBe("rules["+index+"].style.color", "originalText");
+}
+
+shouldBe("rules.length", "2");
+tryMutateSelector(0);
+tryMutateSelector(1);
+tryMutateCSSText(0);
+tryMutateCSSText(1);
+tryMutateProperty(0);
+tryMutateProperty(1);
+tryMutateProperty2(0);
+tryMutateProperty2(1);
+tryRemoveProperty(0);
+tryRemoveProperty(1);
+
+</script>
+<script src=""
+</body>
+</html>
Added: releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash-expected.txt (0 => 167235)
--- releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash-expected.txt 2014-04-14 10:47:30 UTC (rev 167235)
@@ -0,0 +1 @@
+This test passes if it doesn't crash.
Added: releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash.html (0 => 167235)
--- releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash.html (rev 0)
+++ releases/WebKitGTK/webkit-2.4/LayoutTests/fast/css/getMatchedCSSRules-crash.html 2014-04-14 10:47:30 UTC (rev 167235)
@@ -0,0 +1,18 @@
+<html>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+<style>html,tr,img, table,media,body, li, em:nth-child(5){
+ height: 500px
+}
+</style>
+<script>
+function load() {
+ var cssRules = window.getMatchedCSSRules(document.documentElement);
+ cssRules[0].selectorText = 'a,td';
+}
+</script>
+This test passes if it doesn't crash.
+<iframe _onload_=load()>
+</html>
Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog (167234 => 167235)
--- releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog 2014-04-14 10:38:38 UTC (rev 167234)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/ChangeLog 2014-04-14 10:47:30 UTC (rev 167235)
@@ -1,3 +1,52 @@
+2014-03-18 Antti Koivisto <[email protected]>
+
+ Mutating rules returned by getMatchedCSSRules can result in crash
+ https://bugs.webkit.org/show_bug.cgi?id=130209
+
+ Reviewed by Andreas Kling.
+
+ The non-standard getMatchedCSSRules API returns CSSStyleRule objects that don't
+ have parent stylesheet pointer (as we don't know which sheet the rule originated from).
+ Mutating the rule via such wrapper can lead to crashes later as we fail to invalidate
+ the underlying stylesheet.
+
+ Fix by disallowing mutation of style rules that don't have parent sheet pointer. CSSStyleRule
+ has two mutable properties selectorText and style. The latter gives back CSSStyleDeclaration.
+ This patch disallows mutations in both cases for CSSStyleRules that don't have parent stylesheet
+ pointer.
+
+ While it is technically possible to have CSSRules that are legitimately disconnected
+ from stylesheet (by removing rule from sheet while holding a reference to it) it never
+ makes sense to mutate such rule as there is no way to do anything with it afterwards.
+
+ Tests: fast/css/getMatchedCSSProperties-rule-mutation.html
+ fast/css/getMatchedCSSRules-crash.html
+
+ * css/CSSStyleRule.cpp:
+ (WebCore::CSSStyleRule::setSelectorText):
+
+ Bail out if parent stylesheet is null.
+
+ * css/PropertySetCSSStyleDeclaration.cpp:
+ (WebCore::PropertySetCSSStyleDeclaration::setCssText):
+ (WebCore::PropertySetCSSStyleDeclaration::setProperty):
+ (WebCore::PropertySetCSSStyleDeclaration::removeProperty):
+ (WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal):
+
+ Allow StyleRuleCSSStyleDeclaration subclass cancel the mutation via
+ boolean return value from willMutate.
+
+ (WebCore::StyleRuleCSSStyleDeclaration::willMutate):
+
+ Disallow mutation if the owning CSSStyleRule is null or has null stylesheet.
+
+ (WebCore::StyleRuleCSSStyleDeclaration::didMutate):
+
+ We never get here with null rule or stylesheet anymore.
+
+ * css/PropertySetCSSStyleDeclaration.h:
+ (WebCore::PropertySetCSSStyleDeclaration::willMutate):
+
2014-03-13 Andreas Kling <[email protected]>
Don't send synchronous resize events when FrameView has auto-sizing enabled.
Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/css/CSSStyleRule.cpp (167234 => 167235)
--- releases/WebKitGTK/webkit-2.4/Source/WebCore/css/CSSStyleRule.cpp 2014-04-14 10:38:38 UTC (rev 167234)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/css/CSSStyleRule.cpp 2014-04-14 10:47:30 UTC (rev 167235)
@@ -93,6 +93,11 @@
void CSSStyleRule::setSelectorText(const String& selectorText)
{
+ // FIXME: getMatchedCSSRules can return CSSStyleRules that are missing parent stylesheet pointer while
+ // referencing StyleRules that are part of stylesheet. Disallow mutations in this case.
+ if (!parentStyleSheet())
+ return;
+
CSSParser p(parserContext());
CSSSelectorList selectorList;
p.parseSelector(selectorText, selectorList);
Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp (167234 => 167235)
--- releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp 2014-04-14 10:38:38 UTC (rev 167234)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp 2014-04-14 10:47:30 UTC (rev 167235)
@@ -148,7 +148,8 @@
void PropertySetCSSStyleDeclaration::setCssText(const String& text, ExceptionCode& ec)
{
StyleAttributeMutationScope mutationScope(this);
- willMutate();
+ if (!willMutate())
+ return;
ec = 0;
// FIXME: Detect syntax errors and set ec.
@@ -206,10 +207,11 @@
if (!propertyID)
return;
+ if (!willMutate())
+ return;
+
bool important = priority.find("important", 0, false) != notFound;
- willMutate();
-
ec = 0;
bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet());
@@ -229,7 +231,8 @@
if (!propertyID)
return String();
- willMutate();
+ if (!willMutate())
+ return String();
ec = 0;
String result;
@@ -255,7 +258,8 @@
void PropertySetCSSStyleDeclaration::setPropertyInternal(CSSPropertyID propertyID, const String& value, bool important, ExceptionCode& ec)
{
StyleAttributeMutationScope mutationScope(this);
- willMutate();
+ if (!willMutate())
+ return;
ec = 0;
bool changed = m_propertySet->setProperty(propertyID, value, important, contextStyleSheet());
@@ -318,20 +322,24 @@
delete this;
}
-void StyleRuleCSSStyleDeclaration::willMutate()
+bool StyleRuleCSSStyleDeclaration::willMutate()
{
- if (m_parentRule && m_parentRule->parentStyleSheet())
- m_parentRule->parentStyleSheet()->willMutateRules();
+ if (!m_parentRule || !m_parentRule->parentStyleSheet())
+ return false;
+ m_parentRule->parentStyleSheet()->willMutateRules();
+ return true;
}
void StyleRuleCSSStyleDeclaration::didMutate(MutationType type)
{
+ ASSERT(m_parentRule);
+ ASSERT(m_parentRule->parentStyleSheet());
+
if (type == PropertyChanged)
m_cssomCSSValueClones = nullptr;
// Style sheet mutation needs to be signaled even if the change failed. willMutate*/didMutate* must pair.
- if (m_parentRule && m_parentRule->parentStyleSheet())
- m_parentRule->parentStyleSheet()->didMutateRuleFromCSSStyleDeclaration();
+ m_parentRule->parentStyleSheet()->didMutateRuleFromCSSStyleDeclaration();
}
CSSStyleSheet* StyleRuleCSSStyleDeclaration::parentStyleSheet() const
Modified: releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.h (167234 => 167235)
--- releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.h 2014-04-14 10:38:38 UTC (rev 167234)
+++ releases/WebKitGTK/webkit-2.4/Source/WebCore/css/PropertySetCSSStyleDeclaration.h 2014-04-14 10:47:30 UTC (rev 167235)
@@ -74,7 +74,7 @@
protected:
enum MutationType { NoChanges, PropertyChanged };
- virtual void willMutate() { }
+ virtual bool willMutate() WARN_UNUSED_RETURN { return true; }
virtual void didMutate(MutationType) { }
MutableStyleProperties* m_propertySet;
@@ -104,7 +104,7 @@
virtual CSSRule* parentRule() const override { return m_parentRule; }
- virtual void willMutate() override;
+ virtual bool willMutate() override WARN_UNUSED_RETURN;
virtual void didMutate(MutationType) override;
unsigned m_refCount;
