Skip to content

Commit eda53f6

Browse files
davemacaulaydavemacaulay
committed
Merge branch 'MC-10871-xss-html-code' into cms-team-1-delivery
2 parents d822129 + 5766029 commit eda53f6

File tree

16 files changed

+271
-92
lines changed

16 files changed

+271
-92
lines changed

‎app/code/Magento/PageBuilder/Controller/ContentType/Preview.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@
99
namespace Magento\PageBuilder\Controller\ContentType;
1010

1111
use Magento\Framework\Controller\ResultFactory;
12+
use Magento\Framework\App\Action\HttpPostActionInterface;
1213

1314
/**
1415
* Preview controller to render blocks preview on Stage
16+
*
17+
* This isn't placed within the adminhtml folder as it has to extend from the front-end controllers app action to
18+
* ensure the content is rendered in the storefront scope.
19+
*
1520
* @api
1621
*/
17-
class Preview extends \Magento\Framework\App\Action\Action
22+
class Preview extends \Magento\Framework\App\Action\Action implements HttpPostActionInterface
1823
{
1924
/**
2025
* @var \Magento\PageBuilder\Model\Stage\RendererPool

‎app/code/Magento/PageBuilder/Model/Stage/ScriptFilter.php renamed to ‎app/code/Magento/PageBuilder/Model/Stage/HtmlFilter.php

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,19 @@
1111
use Psr\Log\LoggerInterface;
1212

1313
/**
14-
* Filters script tags from stage output
14+
* Filters HTML from stage output
1515
*
1616
* @api
1717
*/
18-
class ScriptFilter
18+
class HtmlFilter
1919
{
2020
/**
2121
* @var LoggerInterface
2222
*/
2323
private $loggerInterface;
2424

2525
/**
26-
* ScriptFilter constructor.
26+
* HtmlFilter constructor.
2727
* @param LoggerInterface $loggerInterface
2828
*/
2929
public function __construct(
@@ -33,12 +33,12 @@ public function __construct(
3333
}
3434

3535
/**
36-
* Remove script tag from html
36+
* Filter HTML text to remove script tags and encode HTML content types
3737
*
3838
* @param string $content
3939
* @return string
4040
*/
41-
public function removeScriptTags(string $content): string
41+
public function filterHtml(string $content): string
4242
{
4343
$dom = new \DOMDocument();
4444
try {
@@ -49,9 +49,25 @@ public function removeScriptTags(string $content): string
4949
$this->loggerInterface->critical($e->getMessage());
5050
}
5151
libxml_use_internal_errors($previous);
52+
// Remove all <script /> tags from output
5253
foreach (iterator_to_array($dom->getElementsByTagName('script')) as $item) {
5354
$item->parentNode->removeChild($item);
5455
}
56+
$xpath = new \DOMXPath($dom);
57+
$htmlContentTypes = $xpath->query('//*[@data-role="html" and not(contains(@class, "placeholder-html-code"))]');
58+
foreach ($htmlContentTypes as $htmlContentType) {
59+
/* @var \DOMElement $htmlContentType */
60+
$innerHTML= '';
61+
$children = $htmlContentType->childNodes;
62+
foreach ($children as $child) {
63+
$innerHTML .= $child->ownerDocument->saveXML($child);
64+
}
65+
$htmlContentType->setAttribute(
66+
"class",
67+
$htmlContentType->getAttribute("class") . " placeholder-html-code"
68+
);
69+
$htmlContentType->nodeValue = htmlentities($innerHTML);
70+
}
5571
return $dom->saveHTML();
5672
}
5773
}

‎app/code/Magento/PageBuilder/Model/Stage/Renderer/CmsStaticBlock.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,28 +33,28 @@ class CmsStaticBlock implements \Magento\PageBuilder\Model\Stage\RendererInterfa
3333
private $loggerInterface;
3434

3535
/**
36-
* @var \Magento\PageBuilder\Model\Stage\ScriptFilter
36+
* @var \Magento\PageBuilder\Model\Stage\HtmlFilter
3737
*/
38-
private $scriptFilter;
38+
private $htmlFilter;
3939

4040
/**
4141
* CmsStaticBlock constructor.
4242
*
4343
* @param \Magento\Cms\Model\ResourceModel\Block\CollectionFactory $blockCollectionFactory
4444
* @param WidgetDirective $widgetDirectiveRenderer
4545
* @param LoggerInterface $loggerInterface
46-
* @param \Magento\PageBuilder\Model\Stage\ScriptFilter $scriptFilter
46+
* @param \Magento\PageBuilder\Model\Stage\HtmlFilter $htmlFilter
4747
*/
4848
public function __construct(
4949
\Magento\Cms\Model\ResourceModel\Block\CollectionFactory $blockCollectionFactory,
5050
WidgetDirective $widgetDirectiveRenderer,
5151
LoggerInterface $loggerInterface,
52-
\Magento\PageBuilder\Model\Stage\ScriptFilter $scriptFilter
52+
\Magento\PageBuilder\Model\Stage\HtmlFilter $htmlFilter
5353
) {
5454
$this->blockCollectionFactory = $blockCollectionFactory;
5555
$this->widgetDirectiveRenderer = $widgetDirectiveRenderer;
5656
$this->loggerInterface = $loggerInterface;
57-
$this->scriptFilter = $scriptFilter;
57+
$this->htmlFilter = $htmlFilter;
5858
}
5959

6060
/**
@@ -96,7 +96,7 @@ public function render(array $params): array
9696

9797
if ($block->isActive()) {
9898
$directiveResult = $this->widgetDirectiveRenderer->render($params);
99-
$result['content'] = $this->scriptFilter->removeScriptTags($directiveResult['content']);
99+
$result['content'] = $this->htmlFilter->filterHtml($directiveResult['content']);
100100
} else {
101101
$result['error'] = __('Block disabled');
102102
}

‎app/code/Magento/PageBuilder/Plugin/Filter/TemplatePlugin.php

Lines changed: 107 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ class TemplatePlugin
2424
*/
2525
private $logger;
2626

27+
/**
28+
* @var \DOMDocument
29+
*/
30+
private $domDocument;
31+
2732
/**
2833
* @param \Psr\Log\LoggerInterface $logger
2934
* @param \Magento\Framework\View\ConfigInterface $viewConfig
@@ -37,6 +42,8 @@ public function __construct(
3742
}
3843

3944
/**
45+
* After filter of template data apply transformations
46+
*
4047
* @param \Magento\Framework\Filter\Template $subject
4148
* @param string $result
4249
*
@@ -45,50 +52,118 @@ public function __construct(
4552
*/
4653
public function afterFilter(\Magento\Framework\Filter\Template $subject, string $result) : string
4754
{
55+
$this->domDocument = false;
56+
4857
// Validate if the filtered result requires background image processing
4958
if (strpos($result, self::DATA_BACKGROUND_IMAGE) !== false) {
50-
$domDocument = new \DOMDocument('1.0', 'UTF-8');
51-
set_error_handler(
52-
function ($errorNumber, $errorString) {
53-
throw new \Exception($errorString, $errorNumber);
54-
}
55-
);
56-
$string = mb_convert_encoding($result, 'HTML-ENTITIES', 'UTF-8');
57-
$wrapperElementId = uniqid();
58-
try {
59-
libxml_use_internal_errors(true);
60-
$domDocument->loadHTML(
61-
'<html><body id="' . $wrapperElementId . '">' . $string . '</body></html>'
62-
);
63-
libxml_clear_errors();
64-
} catch (\Exception $e) {
65-
restore_error_handler();
66-
$this->logger->critical($e);
67-
}
68-
restore_error_handler();
69-
$xpath = new \DOMXPath($domDocument);
59+
$document = $this->getDomDocument($result);
60+
$this->generateBackgroundImageStyles($document);
61+
}
7062

71-
$this->generateBackgroundImageStyles($xpath);
63+
// Process any HTML content types, they need to be decoded on the front-end
64+
if (strpos($result, 'data-role="html"') !== false) {
65+
$document = $this->getDomDocument($result);
66+
$this->decodeHtmlContentTypes($document);
67+
}
7268

69+
// If a document was retrieved we've modified the output so need to retrieve it from within the document
70+
if (isset($document)) {
7371
// Match the contents of the body from our generated document
7472
preg_match(
75-
'/<body id="' . $wrapperElementId . '">(.+)<\/body><\/html>$/si',
76-
$domDocument->saveHTML(),
73+
'/<body>(.+)<\/body><\/html>$/si',
74+
$document->saveHTML(),
7775
$matches
7876
);
7977

8078
return !empty($matches) ? $matches[1] : $result;
8179
}
80+
8281
return $result;
8382
}
8483

84+
/**
85+
* Create a DOM document from a given string
86+
*
87+
* @param string $html
88+
*
89+
* @return \DOMDocument
90+
*/
91+
private function getDomDocument(string $html) : \DOMDocument
92+
{
93+
if (!$this->domDocument) {
94+
$this->domDocument = $this->createDomDocument($html);
95+
}
96+
97+
return $this->domDocument;
98+
}
99+
100+
/**
101+
* Create a DOMDocument from a string
102+
*
103+
* @param string $html
104+
*
105+
* @return \DOMDocument
106+
*/
107+
private function createDomDocument(string $html) : \DOMDocument
108+
{
109+
$domDocument = new \DOMDocument('1.0', 'UTF-8');
110+
set_error_handler(
111+
function ($errorNumber, $errorString) {
112+
throw new \Exception($errorString, $errorNumber);
113+
}
114+
);
115+
$string = mb_convert_encoding($html, 'HTML-ENTITIES', 'UTF-8');
116+
try {
117+
libxml_use_internal_errors(true);
118+
$domDocument->loadHTML(
119+
'<html><body>' . $string . '</body></html>'
120+
);
121+
libxml_clear_errors();
122+
} catch (\Exception $e) {
123+
restore_error_handler();
124+
$this->logger->critical($e);
125+
}
126+
restore_error_handler();
127+
128+
return $domDocument;
129+
}
130+
131+
/**
132+
* Decode the contents of any HTML content types for the store front
133+
*
134+
* @param \DOMDocument $document
135+
*/
136+
private function decodeHtmlContentTypes(\DOMDocument $document): void
137+
{
138+
$xpath = new \DOMXPath($document);
139+
$nodes = $xpath->query('//*[@data-role="html" and not(@data-decoded="true")]');
140+
foreach ($nodes as $node) {
141+
if (strlen(trim($node->nodeValue)) > 0) {
142+
/* @var \DOMElement $node */
143+
$fragment = $document->createDocumentFragment();
144+
$fragment->appendXML($node->nodeValue);
145+
// Store a decoded attribute on the element so we don't double decode
146+
$node->setAttribute("data-decoded", "true");
147+
$node->nodeValue = "";
148+
149+
// If the HTML code in the content type is invalid it may throw
150+
try {
151+
$node->appendChild($fragment);
152+
} catch (\Exception $e) {
153+
$this->logger->critical($e);
154+
}
155+
}
156+
}
157+
}
158+
85159
/**
86160
* Generate the CSS for any background images on the page
87161
*
88-
* @param \DOMXPath $xpath
162+
* @param \DOMDocument $document
89163
*/
90-
private function generateBackgroundImageStyles(\DOMXPath $xpath) : void
164+
private function generateBackgroundImageStyles(\DOMDocument $document) : void
91165
{
166+
$xpath = new \DOMXPath($document);
92167
$nodes = $xpath->query('//*[@' . self:: DATA_BACKGROUND_IMAGE . ']');
93168
foreach ($nodes as $node) {
94169
/* @var \DOMElement $node */
@@ -101,7 +176,7 @@ private function generateBackgroundImageStyles(\DOMXPath $xpath) : void
101176
'style',
102177
$this->generateCssFromImages($elementClass, $images)
103178
);
104-
$style->setAttribute("type", "text/css");
179+
$style->setAttribute('type', 'text/css');
105180
$node->parentNode->appendChild($style);
106181

107182
// Append our new class to the DOM element
@@ -148,14 +223,14 @@ private function generateCssFromImages(string $elementClass, array $images) : st
148223
*/
149224
private function cssFromArray(array $css) : string
150225
{
151-
$output = "";
226+
$output = '';
152227
foreach ($css as $selector => $body) {
153228
if (is_array($body)) {
154-
$output .= $selector . " {";
229+
$output .= $selector . ' {';
155230
$output .= $this->cssFromArray($body);
156-
$output .= "}";
231+
$output .= '}';
157232
} else {
158-
$output .= $selector . ': ' . $body . ";";
233+
$output .= $selector . ': ' . $body . ';';
159234
}
160235
}
161236
return $output;
@@ -173,9 +248,9 @@ private function getMobileMediaQuery() : ?string
173248
'breakpoints/mobile/conditions'
174249
);
175250
if ($breakpoints && count($breakpoints) > 0) {
176-
$mobileBreakpoint = "@media only screen ";
251+
$mobileBreakpoint = '@media only screen ';
177252
foreach ($breakpoints as $key => $value) {
178-
$mobileBreakpoint .= "and (" . $key . ": " . $value . ") ";
253+
$mobileBreakpoint .= 'and (' . $key . ': ' . $value . ') ';
179254
}
180255
return rtrim($mobileBreakpoint);
181256
}

‎app/code/Magento/PageBuilder/Test/Mftf/Section/PageBuilderBlockSection.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<sections xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1010
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Page/etc/SectionObject.xsd">
1111
<section name="BlockOnStage">
12-
<element name="html" type="text" selector="((//div[contains(@class,'pagebuilder-block')])[{{arg1}}]//div[contains(@data-bind,'html: data.main.html')])[{{arg2}}]//a[contains(@class,'pagebuilder-button-primary')]" parameterized="true"/>
12+
<element name="html" type="text" selector="((//div[contains(@class,'pagebuilder-block')])[{{arg1}}]//div[contains(@data-bind,'html: data.main.html')])[{{arg2}}]//div[contains(@class,'placeholder-html-code')]" parameterized="true"/>
1313
<element name="status" type="text" selector="((//div[contains(@class,'pagebuilder-block')])[{{arg1}}]//span[contains(@class,'placeholder') and text()='{{arg}}'])" parameterized="true"/>
1414
<element name="deleted" type="text" selector="((//div[contains(@class,'pagebuilder-block')])[{{arg1}}]//span[contains(@class,'placeholder') and contains(text(),'Block with ID: {{arg}} doesn')])" parameterized="true"/>
1515
<element name="title" type="text" selector="(//div[contains(@class,'pagebuilder-block')])[{{arg1}}]//div[contains(@class,'pagebuilder-options-wrapper')]//div[contains(@class,'option-title') and text()='{{arg}}']" parameterized="true"/>

‎app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderBlockTest.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@
172172
<comment userInput="Validate stage after updating block" stepKey="validateStage2" />
173173
<actionGroup ref="switchToPageBuilderStage" stepKey="switchToPageBuilderStage"/>
174174
<waitForElementVisible selector="{{HtmlOnStage.base('1')}}" stepKey="waitForHtmlBaseStage1"/>
175-
<waitForElementVisible selector="{{BlockOnStage.html('1', '1')}}" stepKey="waitForHtmlStage1"/>
175+
<see selector="{{BlockOnStage.html('1', '1')}}" userInput="{{PageBuilderHtmlPropertyButton.value}}" stepKey="waitForHtmlStage1"/>
176176
<actionGroup ref="ClearCacheActionGroup" stepKey="clearMagentoCache"/>
177177
<!-- Validate Storefront -->
178178
<comment userInput="Validate storefront after updating block" stepKey="validateStorefront2" />

‎app/code/Magento/PageBuilder/view/adminhtml/pagebuilder/content_type/html.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
<style name="padding" storage_key="margins_and_padding" reader="Magento_PageBuilder/js/property/paddings" converter="Magento_PageBuilder/js/converter/style/paddings"/>
3535
<attribute name="name" source="data-role"/>
3636
<attribute name="appearance" source="data-appearance"/>
37-
<html name="html"/>
37+
<html name="html" converter="Magento_PageBuilder/js/converter/html/decode"/>
3838
<css name="css_classes"/>
3939
</element>
4040
</elements>

‎app/code/Magento/PageBuilder/view/adminhtml/ui_component/pagebuilder_html_form.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
<dataScope>html</dataScope>
8989
<dataType>text</dataType>
9090
<placeholder translate="true">Enter HTML, CSS or JavaScript code</placeholder>
91+
<notice translate="true">HTML code must be valid, all CSS &amp; JavaScript must be wrapped in their HTML elements.</notice>
9192
</settings>
9293
</field>
9394
</fieldset>

0 commit comments

Comments
 (0)