Skip to content
Permalink
Browse files
Make SVGAnimateElement respect javascriptMarkupEnabled
https://bugs.webkit.org/show_bug.cgi?id=241820
<rdar://92834618 >

Reviewed by Said Abou-Hallawa.

from/to/values attributes should be stripped in presence of javascript URLs with the pref on.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::attributeContainsJavascriptURL const):
(WebCore::Element::stripScriptingAttributes const):
(WebCore::Element::isJavaScriptURLAttribute const): Deleted.
* Source/WebCore/dom/Element.h:
* Source/WebCore/editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplacementFragment::removeContentsWithSideEffects):
* Source/WebCore/editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::appendStartTag):
* Source/WebCore/svg/SVGAnimationElement.cpp:
(WebCore::SVGAnimationElement::attributeContainsJavascriptURL const):
* Source/WebCore/svg/SVGAnimationElement.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewConfiguration.mm:
(TEST):

Canonical link: https://commits.webkit.org/251752@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295747 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
nt1m committed Jun 22, 2022
1 parent 6c003d7 commit 431f4db465c43b419910c6889d60edb9454a7de5
Showing 7 changed files with 44 additions and 5 deletions.
@@ -2315,7 +2315,7 @@ bool Element::isEventHandlerAttribute(const Attribute& attribute) const
return attribute.name().namespaceURI().isNull() && attribute.name().localName().startsWith("on"_s);
}

bool Element::isJavaScriptURLAttribute(const Attribute& attribute) const
bool Element::attributeContainsJavascriptURL(const Attribute& attribute) const

This comment has been minimized.

Copy link
@darinadler

darinadler Jun 22, 2022

Member

JavaScript has a capital "S".

This comment has been minimized.

Copy link
@nt1m

nt1m Jun 22, 2022

Author Member
{
return isURLAttribute(attribute) && WTF::protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(attribute.value()));
}
@@ -2324,7 +2324,7 @@ void Element::stripScriptingAttributes(Vector<Attribute>& attributeVector) const
{
attributeVector.removeAllMatching([this](auto& attribute) -> bool {
return this->isEventHandlerAttribute(attribute)
|| this->isJavaScriptURLAttribute(attribute)
|| this->attributeContainsJavascriptURL(attribute)
|| this->isHTMLContentAttribute(attribute);
});
}
@@ -294,7 +294,7 @@ class Element : public ContainerNode {
void parserSetAttributes(const Vector<Attribute>&);

bool isEventHandlerAttribute(const Attribute&) const;
bool isJavaScriptURLAttribute(const Attribute&) const;
virtual bool attributeContainsJavascriptURL(const Attribute&) const;

// Remove attributes that might introduce scripting from the vector leaving the element unchanged.
void stripScriptingAttributes(Vector<Attribute>&) const;
@@ -238,7 +238,7 @@ void ReplacementFragment::removeContentsWithSideEffects()
}
if (element->hasAttributes()) {
for (auto& attribute : element->attributesIterator()) {
if (element->isEventHandlerAttribute(attribute) || element->isJavaScriptURLAttribute(attribute))
if (element->isEventHandlerAttribute(attribute) || element->attributeContainsJavascriptURL(attribute))
attributesToRemove.append({ element.copyRef(), attribute.name() });
}
}
@@ -556,7 +556,7 @@ void StyledMarkupAccumulator::appendStartTag(StringBuilder& out, const Element&
// We'll handle the style attribute separately, below.
if (attribute.name() == styleAttr && shouldOverrideStyleAttr)
continue;
if (element.isEventHandlerAttribute(attribute) || element.isJavaScriptURLAttribute(attribute))
if (element.isEventHandlerAttribute(attribute) || element.attributeContainsJavascriptURL(attribute))
continue;
#if ENABLE(DATA_DETECTION)
if (replacementType == SpanReplacementType::DataDetector && DataDetection::isDataDetectorAttribute(attribute.name()))
@@ -31,6 +31,7 @@
#include "CSSPropertyParser.h"
#include "Document.h"
#include "FloatConversion.h"
#include "HTMLParserIdioms.h"
#include "RenderObject.h"
#include "SVGAnimateColorElement.h"
#include "SVGAnimateElement.h"
@@ -147,6 +148,21 @@ bool SVGAnimationElement::isSupportedAttribute(const QualifiedName& attrName)
return supportedAttributes.get().contains<SVGAttributeHashTranslator>(attrName);
}

bool SVGAnimationElement::attributeContainsJavascriptURL(const Attribute& attribute) const
{
if (attribute.name() == SVGNames::fromAttr || attribute.name() == SVGNames::toAttr)
return WTF::protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(attribute.value()));

if (attribute.name() == SVGNames::valuesAttr) {
for (auto innerValue : StringView(attribute.value()).split(';')) {
if (WTF::protocolIsJavaScript(innerValue.stripWhiteSpace()))

This comment has been minimized.

Copy link
@darinadler

darinadler Jun 22, 2022

Member

I believe that the proper stripping of leading whitespace for an HTML or SVG attribute value is not the StringView::stripWhiteSpace function, but rather the stripLeadingAndTrailingHTMLSpaces function. You can see that in the renamed Element::isJavaScriptURLAttribute function above, and also in HTMLFrameElementBase::setLocation. Ideally I don’t think we want subtly different definitions of what characters should be stripped.

This comment has been minimized.

Copy link
@nt1m

nt1m Jun 22, 2022

Author Member

#1700

@darinadler Can you please review it?

return true;
}
return false;
}
return Element::attributeContainsJavascriptURL(attribute);
}

void SVGAnimationElement::parseAttribute(const QualifiedName& name, const AtomString& value)
{
if (name == SVGNames::valuesAttr) {
@@ -88,6 +88,7 @@ class SVGAnimationElement : public SVGSMILElement, public SVGTests {
virtual void resetAnimation();

static bool isSupportedAttribute(const QualifiedName&);
bool attributeContainsJavascriptURL(const Attribute&) const final;
void parseAttribute(const QualifiedName&, const AtomString&) override;
void svgAttributeChanged(const QualifiedName&) override;

@@ -172,3 +172,25 @@ HTTPServer server([&] (Connection connection) {
NSString *bodyHTML = [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"];
EXPECT_WK_STREQ(bodyHTML, @"<table></table>");
}

TEST(WebKit, ConfigurationDisableJavaScriptSVGAnimateElement)
{
auto configuration = adoptNS([WKWebViewConfiguration new]);
EXPECT_TRUE([configuration _allowsJavaScriptMarkup]);
[configuration _setAllowsJavaScriptMarkup:NO];
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 200) configuration:configuration.get()]);
[webView synchronouslyLoadHTMLString:@"<svg><a><rect fill=\"green\" width=\"10\" height=\"10\"></rect><animate attributeName=\"href\" to=\"javascript:document.write('FAIL')\"/></a></svg>"];
NSString *bodyHTML = [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"];
EXPECT_WK_STREQ(bodyHTML, @"<svg><a><rect fill=\"green\" width=\"10\" height=\"10\"></rect><animate attributeName=\"href\"></animate></a></svg>");
}

TEST(WebKit, ConfigurationDisableJavaScriptSVGAnimateElementComplex)
{
auto configuration = adoptNS([WKWebViewConfiguration new]);
EXPECT_TRUE([configuration _allowsJavaScriptMarkup]);
[configuration _setAllowsJavaScriptMarkup:NO];
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:configuration.get()]);
[webView synchronouslyLoadHTMLString:@"<svg><a><rect fill=\"green\" width=\"10\" height=\"10\"></rect><animate attributeName=\"href\" dur=\"4s\" calcMode=\"spline\" repeatCount=\"indefinite\" values=\"javascript:document.write('FAIL'); javascript:document.write('FAIL'); javascript:document.write(location.href); javascript:document.write('FAIL'); javascript:document.write('FAIL')\" keyTimes=\"0; 0.25; 0.5; 0.75; 1\" keySplines=\"0.5 0 0.5 1; 0.5 0 0.5 1; 0.5 0 0.5 1; 0.5 0 0.5 1\"/></a></svg>"];
NSString *bodyHTML = [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"];
EXPECT_WK_STREQ(bodyHTML, @"<svg><a><rect fill=\"green\" width=\"10\" height=\"10\"></rect><animate attributeName=\"href\" dur=\"4s\" calcMode=\"spline\" repeatCount=\"indefinite\" keyTimes=\"0; 0.25; 0.5; 0.75; 1\" keySplines=\"0.5 0 0.5 1; 0.5 0 0.5 1; 0.5 0 0.5 1; 0.5 0 0.5 1\"></animate></a></svg>");
}

0 comments on commit 431f4db

Please sign in to comment.