Merge pull request silverstripe#10 from silverstripe-security/fix/cve…

…-2019-19325 [CVE-2019-19325] XSS through non-scalar FormField attributes
unclecheese · Feb 16, 2020 · bcb6ac9 · bcb6ac9
2 parents d2a55f2 + d515e5e
commit bcb6ac9
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 1 deletion.
diff --git a/code/CMSMenuItem.php b/code/CMSMenuItem.php
@@ -108,7 +108,11 @@ public function getAttributesHTML($attrs = null)
         $parts = array();
 
         foreach ($attrs as $name => $value) {
-            $parts[] = ($value === true) ? "{$name}=\"{$name}\"" : "{$name}=\"" . Convert::raw2att($value) . "\"";
+            if ($value === true) {
+                $value = $name;
+            }
+
+            $parts[] = sprintf('%s="%s"', Convert::raw2att($name), Convert::raw2att($value));
         }
 
         /** @var DBHTMLText $fragment */

diff --git a/tests/php/CMSMenuItemTest.php b/tests/php/CMSMenuItemTest.php
@@ -43,5 +43,8 @@ public function testAttributes()
             (string)$menuItem->getAttributesHTML('data-foo'),
             'getAttributesHTML() ignores a string argument and falls back to class property'
         );
+
+        $menuItem->setAttributes(['<html>' => '<html>']);
+        $this->assertNotContains('<html>', $menuItem->getAttributesHTML(), 'Html is escaped for both name and value');
     }
 }