Merge remote-tracking branch 'remotes/origin/MC-15311' into cms-team-1-delivery

Author: danmooney2

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

@@ -7,6 +7,8 @@
 
 namespace Magento\PageBuilder\Plugin\Filter;
 
+use Magento\Store\Model\Store;
+
 /**
  * Plugin to the template filter to process any background images added by Page Builder
  */
@@ -105,6 +107,32 @@ public function afterFilter(\Magento\Framework\Filter\Template $subject, string
         return $result;
     }
 
+    /**
+     * Determine if custom variable directive's return value needs to be escaped and do so if true
+     *
+     * @param \Magento\Framework\Filter\Template $subject
+     * @param \Closure $proceed
+     * @param string[] $construction
+     * @return string
+     */
+    public function aroundCustomvarDirective(
+        \Magento\Framework\Filter\Template $subject,
+        \Closure $proceed,
+        $construction
+    ) {
+        // Determine the need to escape the return value of observed method.
+        // Admin context requires store ID of 0; in that context return value should be escaped
+        $shouldEscape = $subject->getStoreId() !== null && (int) $subject->getStoreId() === Store::DEFAULT_STORE_ID;
+
+        if (!$shouldEscape) {
+            return $proceed($construction);
+        }
+
+        $result = $proceed($construction);
+
+        return htmlspecialchars($result);
+    }
+
     /**
      * Create a DOM document from a given string
      *
@@ -180,8 +208,27 @@ private function generateDecodedHtmlPlaceholderMappingInDocument(\DOMDocument $d
                 continue;
             }
 
+            // clone html code content type to save reference to its attributes/outerHTML, which we are not going to
+            // decode
+            $clonedHtmlContentTypeNode = clone $htmlContentTypeNode;
+
+            // clear inner contents of cloned node for replacement later with $decodedInnerHtml using sprintf;
+            // we want to retain html content type node and avoid doing any manipulation on it
+            $clonedHtmlContentTypeNode->nodeValue = '%s';
+
+            // remove potentially harmful attributes on html content type node itself
+            while ($htmlContentTypeNode->attributes->length) {
+                $htmlContentTypeNode->removeAttribute($htmlContentTypeNode->attributes->item(0)->name);
+            }
+
+            // decode outerHTML safely
             $preDecodedOuterHtml = $document->saveHTML($htmlContentTypeNode);
-            $decodedOuterHtml = html_entity_decode($preDecodedOuterHtml);
+
+            // clear empty <div> wrapper around outerHTML to replace with $clonedHtmlContentTypeNode
+            $decodedInnerHtml = preg_replace('#^<[^>]*>|</[^>]*>$#', '', html_entity_decode($preDecodedOuterHtml));
+
+            // Use $clonedHtmlContentTypeNode's placeholder to inject decoded inner html
+            $decodedOuterHtml = sprintf($document->saveHTML($clonedHtmlContentTypeNode), $decodedInnerHtml);
 
             // generate unique node name element to replace with decoded html contents at end of processing;
             // goal is to create a document as few times as possible to prevent inadvertent parsing of contents as html

app/code/Magento/PageBuilder/Test/Mftf/Data/AdvancedData.xml

@@ -304,6 +304,20 @@
         <data key="fieldName">css_classes</data>
         <data key="value">first-class second-class third-class</data>
     </entity>
+    <entity name="PageBuilderAdvancedCssClassesProperty_Invalid_GreaterThan" type="pagebuilder_advanced_css_classes_property">
+        <data key="name">CSS Classes</data>
+        <data key="section">advanced</data>
+        <data key="fieldName">css_classes</data>
+        <data key="value">first&gt;class second-class third-class</data>
+        <data key="errorMessage">Please enter a valid CSS class.</data>
+    </entity>
+    <entity name="PageBuilderAdvancedCssClassesProperty_Invalid_LessThan" type="pagebuilder_advanced_css_classes_property">
+        <data key="name">CSS Classes</data>
+        <data key="section">advanced</data>
+        <data key="fieldName">css_classes</data>
+        <data key="value">first-class second-class third&lt;class</data>
+        <data key="errorMessage">Please enter a valid CSS class.</data>
+    </entity>
     <!-- Text Color -->
     <entity name="PageBuilderAdvancedTextColorProperty" type="pagebuilder_advanced_color_property">
         <data key="name">Text Color</data>

app/code/Magento/PageBuilder/Test/Mftf/Test/AdminPageBuilderAdvancedConfigurationTest.xml

@@ -1629,8 +1629,6 @@
         <actionGroup ref="dragContentTypeToStage" stepKey="dragRowIntoStage2">
             <argument name="contentType" value="PageBuilderRowContentType"/>
         </actionGroup>
-        <!-- Add CSS Classes: Empty -->
-        <comment userInput="Add CSS Classes: Empty" stepKey="commentAddCSSClassesEmpty"/>
         <actionGroup ref="expandPageBuilderPanelMenuSection" stepKey="expandPageBuilderPanelMenuSection">
             <argument name="contentType" value="PageBuilderTextContentType"/>
         </actionGroup>
@@ -1648,6 +1646,24 @@
         <actionGroup ref="addTextToTinyMCE" stepKey="enterText1">
             <argument name="property" value="PageBuilderTextProperty"/>
         </actionGroup>
+        <!-- Add CSS Classes: Invalid Greater Than -->
+        <comment userInput="Add CSS Classes: Invalid Greater Than" stepKey="commentAddCSSClassesInvalidGreaterThan"/>
+        <actionGroup ref="fillSlideOutPanelFieldAndExpectToSeeErrorInFieldset" stepKey="enterGreaterThanSymbol">
+            <argument name="property" value="PageBuilderAdvancedCssClassesProperty_Invalid_GreaterThan"/>
+        </actionGroup>
+        <actionGroup ref="saveEditPanelAndValidateFieldError" stepKey="validateErrorGreaterThan">
+            <argument name="property" value="PageBuilderAdvancedCssClassesProperty_Invalid_GreaterThan"/>
+        </actionGroup>
+        <!-- Add CSS Classes: Invalid Less Than -->
+        <comment userInput="Add CSS Classes: Invalid Less Than" stepKey="commentAddCSSClassesInvalidLessThan"/>
+        <actionGroup ref="fillSlideOutPanelFieldAndExpectToSeeErrorInFieldset" stepKey="enterLessThanSymbol">
+            <argument name="property" value="PageBuilderAdvancedCssClassesProperty_Invalid_LessThan"/>
+        </actionGroup>
+        <actionGroup ref="saveEditPanelAndValidateFieldError" stepKey="validateErrorLessThan">
+            <argument name="property" value="PageBuilderAdvancedCssClassesProperty_Invalid_LessThan"/>
+        </actionGroup>
+        <!-- Add CSS Classes: Empty -->
+        <comment userInput="Add CSS Classes: Empty" stepKey="commentAddCSSClassesEmpty"/>
         <actionGroup ref="clearSlideOutPanelFieldGeneral" stepKey="clearCSSClasses">
             <argument name="property" value="PageBuilderAdvancedCssClassesDefaultProperty"/>
         </actionGroup>

app/code/Magento/PageBuilder/view/adminhtml/ui_component/pagebuilder_base_form.xml

@@ -184,6 +184,9 @@
                 <dataType>text</dataType>
                 <label translate="true">CSS Classes</label>
                 <notice translate="true">Space separated list of classes.</notice>
+                <validation>
+                    <rule name="validate-css-class" xsi:type="boolean">true</rule>
+                </validation>
             </settings>
         </field>
         <field name="margins_and_padding" sortOrder="70" formElement="input" component="Magento_PageBuilder/js/form/element/margins-and-padding">

app/code/Magento/PageBuilder/view/adminhtml/web/js/form/element/validator-rules-mixin.js

@@ -52,6 +52,15 @@ define([
         return (/<a[\s]+([^>]+)>|<a>|<\/a>/igm).test(str);
     }
 
+    /**
+     * Validate that string is a proper css-class
+     * @param {String} str
+     * @return {Boolean}
+     */
+    function validateCssClass(str) {
+        return (/^[a-zA-Z _\-\d]+$/i).test(str);
+    }
+
     /**
      * Validate message field and url field anchor tag is used exclusively by one field
      * @param {String} message
@@ -147,6 +156,18 @@ define([
             $.mage.__('Please enter a valid video URL.')
         );
 
+        validator.addRule(
+            'validate-css-class',
+            function (value) {
+                if (utils.isEmptyNoTrim(value)) {
+                    return true;
+                }
+
+                return validateCssClass(value);
+            },
+            $.mage.__('Please enter a valid CSS class.')
+        );
+
         validator.addRule(
             'required-entry',
             function (value) {

dev/tests/integration/testsuite/Magento/PageBuilder/Plugin/Filter/TemplatePluginTest.php

@@ -8,6 +8,7 @@
 
 namespace Magento\PageBuilder\Plugin\Filter;
 
+use Magento\Store\Model\Store;
 use Magento\Widget\Model\Template\Filter as TemplateFilter;
 use Magento\TestFramework\Helper\Bootstrap;
 
@@ -30,13 +31,18 @@ protected function setUp()
     {
         $this->objectManager = Bootstrap::getObjectManager();
         $this->templateFilter = $this->objectManager->get(TemplateFilter::class);
+
+        // set store id to 0 to recognize that escaping is required in custom variable
+        $this->templateFilter->setStoreId(Store::DEFAULT_STORE_ID);
     }
 
     /**
      * @param string $preFiltered
      * @param string $postFiltered
      * @param string $preFilteredBasename
      * @dataProvider filterDataProvider
+     * @magentoDataFixture Magento/PageBuilder/_files/custom_variable_xss.php
+     * @magentoDbIsolation enabled
      */
     public function testFiltering(string $preFiltered, string $postFiltered, string $preFilteredBasename)
     {

dev/tests/integration/testsuite/Magento/PageBuilder/_files/custom_variable_xss.php

@@ -0,0 +1,22 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+
+/** @var \Magento\Variable\Model\ResourceModel\Variable $variableResource */
+$variableResource = $this->objectManager->get(\Magento\Variable\Model\ResourceModel\Variable::class);
+
+/** @var \Magento\Variable\Model\Variable $variable */
+$variable = $this->objectManager->get(\Magento\Variable\Model\Variable::class);
+
+$variable->setData([
+    'code' => 'xssVariable',
+    'name' => 'xssVariable',
+    'html_value' => '<img src=x onerror="alert(0)">',
+    'plain_value' => '<img src=x onerror="alert(0)">',
+]);
+
+$variableResource->save($variable);

dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/custom_var_directive_post_filter.html

@@ -0,0 +1 @@
+<img src=x onerror="alert(0)">

dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/custom_var_directive_pre_filter.html

@@ -0,0 +1 @@
+{{customVar code=xssVariable}}

dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_encoded_content_on_outer_node_post_filter.html

@@ -0,0 +1,3 @@
+<div data-content-type="html" class="&gt;'&gt;&quot;&gt;&lt;img src=x onerror=alert(0)&gt;" data-decoded="true">
+    Nothing to see here
+</div>

dev/tests/integration/testsuite/Magento/PageBuilder/_files/template_plugin/html_content_type_encoded_content_on_outer_node_pre_filter.html

@@ -0,0 +1,3 @@
+<div data-content-type="html" class="&gt;'&gt;&quot;&gt;&lt;img src=x onerror=alert(0)&gt;">
+    Nothing to see here
+</div>