AC-9460: Stored XSS fix via PayPal authentication certificate

Author: faraz-gl

app/code/Magento/Config/Block/System/Config/Form/Field/File.php

@@ -9,16 +9,43 @@
  *
  * @author     Magento Core Team <core@magentocommerce.com>
  */
+declare(strict_types=1);
+
 namespace Magento\Config\Block\System\Config\Form\Field;
 
+use Magento\Framework\Data\Form\Element\CollectionFactory;
+use Magento\Framework\Escaper;
+use Magento\Framework\Data\Form\Element\Factory;
+
 class File extends \Magento\Framework\Data\Form\Element\File
 {
+    /**
+     * @var Escaper
+     */
+    private Escaper $escaper;
+
+    /**
+     * @param Factory $factoryElement
+     * @param CollectionFactory $factoryCollection
+     * @param Escaper $escaper
+     * @param array $data
+     */
+    public function __construct(
+        Factory $factoryElement,
+        CollectionFactory $factoryCollection,
+        Escaper $escaper,
+        array $data = []
+    ) {
+        $this->escaper = $escaper;
+        parent::__construct($factoryElement, $factoryCollection, $this->escaper, $data);
+    }
+
     /**
      * Get element html
      *
      * @return string
      */
-    public function getElementHtml()
+    public function getElementHtml(): string
     {
         $html = parent::getElementHtml();
         $html .= $this->_getDeleteCheckbox();
@@ -30,12 +57,12 @@ public function getElementHtml()
      *
      * @return string
      */
-    protected function _getDeleteCheckbox()
+    protected function _getDeleteCheckbox(): string
     {
         $html = '';
         if ((string)$this->getValue()) {
             $label = __('Delete File');
-            $html .= '<div>' . $this->getValue() . ' ';
+            $html .= '<div>' . $this->escaper->escapeHtml($this->getValue()) . ' ';
             $html .= '<input type="checkbox" name="' .
                 parent::getName() .
                 '[delete]" value="1" class="checkbox" id="' .

app/code/Magento/Config/Test/Unit/Block/System/Config/Form/Field/FileTest.php

@@ -8,49 +8,85 @@
 namespace Magento\Config\Test\Unit\Block\System\Config\Form\Field;
 
 use Magento\Config\Block\System\Config\Form\Field\File;
+use Magento\Framework\Data\Form\Element\Factory;
+use Magento\Framework\Data\Form\Element\CollectionFactory;
 use Magento\Framework\DataObject;
 use Magento\Framework\Escaper;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 /**
  * Tests for \Magento\Framework\Data\Form\Field\File
  */
 class FileTest extends TestCase
 {
+    /**
+     * XSS value
+     */
+    private const XSS_FILE_NAME_TEST = '<img src=x onerror=alert(1)>.crt';
+
+    /**
+     * Input name
+     */
+    private const INPUT_NAME_TEST = 'test_name';
+
     /**
      * @var File
      */
     protected $file;
 
+    /**
+     * @var Factory|MockObject
+     */
+    private $factoryMock;
+
+    /**
+     * @var CollectionFactory|MockObject
+     */
+    private $factoryCollectionMock;
+
+    /**
+     * @var Escaper|MockObject
+     */
+    private $escaperMock;
+
     /**
      * @var array
      */
-    protected $testData;
+    protected array $testData = [
+        'before_element_html' => 'test_before_element_html',
+        'html_id' => 'test_id',
+        'name' => 'test_name',
+        'value' => 'test_value',
+        'title' => 'test_title',
+        'disabled' => true,
+        'after_element_js' => 'test_after_element_js',
+        'after_element_html' => 'test_after_element_html',
+        'html_id_prefix' => 'test_id_prefix_',
+        'html_id_suffix' => '_test_id_suffix',
+    ];
 
     protected function setUp(): void
     {
         $objectManager = new ObjectManager($this);
 
-        $this->testData = [
-            'before_element_html' => 'test_before_element_html',
-            'html_id' => 'test_id',
-            'name' => 'test_name',
-            'value' => 'test_value',
-            'title' => 'test_title',
-            'disabled' => true,
-            'after_element_js'    => 'test_after_element_js',
-            'after_element_html'  => 'test_after_element_html',
-            'html_id_prefix'      => 'test_id_prefix_',
-            'html_id_suffix'      => '_test_id_suffix',
-        ];
-
+        $this->factoryMock = $this->getMockBuilder(Factory::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->factoryCollectionMock = $this->getMockBuilder(CollectionFactory::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->escaperMock = $this->getMockBuilder(Escaper::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->file = $objectManager->getObject(
             File::class,
             [
-                '_escaper' => $objectManager->getObject(Escaper::class),
+                'factoryElement' => $this->factoryMock,
+                'factoryCollection' => $this->factoryCollectionMock,
+                '_escaper' => $this->escaperMock,
                 'data' => $this->testData,
-
             ]
         );
 
@@ -60,13 +96,20 @@ protected function setUp(): void
         $this->file->setForm($formMock);
     }
 
-    public function testGetElementHtml()
+    public function testGetElementHtml(): void
     {
-        $html = $this->file->getElementHtml();
-
         $expectedHtmlId = $this->testData['html_id_prefix']
             . $this->testData['html_id']
             . $this->testData['html_id_suffix'];
+        $this->escaperMock->expects($this->any())->method('escapeHtml')->willReturnMap(
+            [
+                [$expectedHtmlId, null, $expectedHtmlId],
+                [self::XSS_FILE_NAME_TEST, null, self::XSS_FILE_NAME_TEST],
+                [self::INPUT_NAME_TEST, null, self::INPUT_NAME_TEST],
+            ]
+        );
+
+        $html = $this->file->getElementHtml();
 
         $this->assertStringContainsString('<label class="addbefore" for="' . $expectedHtmlId . '"', $html);
         $this->assertStringContainsString($this->testData['before_element_html'], $html);