Skip to content

Commit 42577bc

Browse files
committed
AC-11446: allowed tags updated for CMS content
1 parent 4ff8b0f commit 42577bc

File tree

3 files changed

+82
-23
lines changed

3 files changed

+82
-23
lines changed

app/design/adminhtml/Magento/backend/i18n/en_US.csv

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,3 +548,5 @@ Dashboard,Dashboard
548548
"Store Email Addresses Section","Store Email Addresses Section"
549549
"Email to a Friend","Email to a Friend"
550550
"Invalid data type","Invalid data type"
551+
"Invalid value provided for attribute %1","Invalid value provided for attribute %1"
552+

lib/internal/Magento/Framework/Test/Unit/Validator/HTML/ConfigurableWYSIWYGValidatorTest.php

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class ConfigurableWYSIWYGValidatorTest extends TestCase
2626
public function getConfigurations(): array
2727
{
2828
return [
29-
'no-html' => [['div'], [], [], 'just text', true, [], []],
30-
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', true, [], []],
29+
'no-html' => [['div'], [], [], 'just text', false, [], []],
30+
'allowed-tag' => [['div'], [], [], 'just text and <div>a div</div>', false, [], []],
3131
'restricted-tag' => [
3232
['div', 'p'],
3333
[],
@@ -165,6 +165,24 @@ public function getConfigurations(): array
165165
true,
166166
[],
167167
['div' => ['src' => false]]
168+
],
169+
'invalid-allowed-tag-attributes' => [
170+
['a'],
171+
['href'],
172+
['a' => ['href']],
173+
'<a href="javascript:alert(1)">a</a>',
174+
false,
175+
[],
176+
[]
177+
],
178+
'allowed-empty-tag' => [
179+
[],
180+
[],
181+
[],
182+
'',
183+
false,
184+
[],
185+
[]
168186
]
169187
];
170188
}
@@ -224,20 +242,23 @@ function (string $givenTag, array $attrs) use ($tag, $allowedAttributes): void {
224242
);
225243
$tagValidatorsMocks[$tag] = [$mock];
226244
}
227-
$validator = new ConfigurableWYSIWYGValidator(
228-
$allowedTags,
229-
$allowedAttr,
230-
$allowedTagAttrs,
231-
$attrValidators,
232-
$tagValidatorsMocks
233-
);
234-
$valid = true;
235245
try {
236-
$validator->validate($html);
237-
} catch (ValidationException $exception) {
246+
$validator = new ConfigurableWYSIWYGValidator(
247+
$allowedTags,
248+
$allowedAttr,
249+
$allowedTagAttrs,
250+
$attrValidators,
251+
$tagValidatorsMocks
252+
);
253+
$valid = true;
254+
try {
255+
$validator->validate($html);
256+
} catch (ValidationException $exception) {
257+
$valid = false;
258+
}
259+
} catch (\InvalidArgumentException $exception) {
238260
$valid = false;
239261
}
240-
241262
self::assertEquals($isValid, $valid);
242263
}
243264
}

lib/internal/Magento/Framework/Validator/HTML/ConfigurableWYSIWYGValidator.php

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,19 @@
1515
*/
1616
class ConfigurableWYSIWYGValidator implements WYSIWYGValidatorInterface
1717
{
18+
/**
19+
* @var string
20+
*/
21+
private static string $xssFiltrationPattern =
22+
'/((javascript(\\\\x3a|:|%3A))|(data(\\\\x3a|:|%3A))|(vbscript:)|(script)|(alert\())|'
23+
. '((\\\\x6A\\\\x61\\\\x76\\\\x61\\\\x73\\\\x63\\\\x72\\\\x69\\\\x70\\\\x74(\\\\x3a|:|%3A))|'
24+
. '(\\\\x64\\\\x61\\\\x74\\\\x61(\\\\x3a|:|%3A)))/i';
25+
26+
/**
27+
* @var string
28+
*/
29+
private static string $contentFiltrationPattern = "/(<body)/i";
30+
1831
/**
1932
* @var string[]
2033
*/
@@ -84,6 +97,7 @@ public function validate(string $content): void
8497
$this->validateConfigured($xpath);
8598
$this->callAttributeValidators($xpath);
8699
$this->callTagValidators($xpath);
100+
$this->validateAttributeValue($xpath);
87101
}
88102

89103
/**
@@ -96,18 +110,19 @@ public function validate(string $content): void
96110
private function validateConfigured(\DOMXPath $xpath): void
97111
{
98112
//Validating tags
113+
$this->allowedTags = array_merge($this->allowedTags, ["body", "html"]);
99114
$found = $xpath->query(
100115
'//*['
101-
. implode(
102-
' and ',
103-
array_map(
104-
function (string $tag): string {
105-
return "name() != '$tag'";
106-
},
107-
array_merge($this->allowedTags, ['body', 'html'])
108-
)
116+
. implode(
117+
' and ',
118+
array_map(
119+
function (string $tag): string {
120+
return "name() != '$tag'";
121+
},
122+
$this->allowedTags
109123
)
110-
.']'
124+
)
125+
.']'
111126
);
112127
if (count($found)) {
113128
throw new ValidationException(
@@ -234,12 +249,33 @@ function () use (&$loaded) {
234249
$loaded = false;
235250
}
236251
);
237-
$loaded = $dom->loadHTML("<html><body>$content</body></html>");
252+
$matches = [];
253+
preg_match_all(self::$contentFiltrationPattern, $content, $matches);
254+
$loaded = !(count($matches[0]) > 1) && $dom->loadHTML($content, LIBXML_HTML_NOIMPLIED);
238255
restore_error_handler();
239256
if (!$loaded) {
240257
throw new ValidationException(__('Invalid HTML content provided'));
241258
}
242259

243260
return $dom;
244261
}
262+
263+
/**
264+
* Validate values of html attributes
265+
*
266+
* @param \DOMXPath $xpath
267+
* @return void
268+
* @throws ValidationException
269+
*/
270+
private function validateAttributeValue(\DOMXPath $xpath): void
271+
{
272+
$nodes = $xpath->query('//@*');
273+
foreach ($nodes as $node) {
274+
if (preg_match(self::$xssFiltrationPattern, $node->parentNode->getAttribute($node->nodeName))) {
275+
throw new ValidationException(
276+
__('Invalid value provided for attribute %1', $node->nodeName)
277+
);
278+
}
279+
}
280+
}
245281
}

0 commit comments

Comments
 (0)