Skip to content
Snippets Groups Projects
Commit 0217ee0a authored by catch's avatar catch
Browse files

Issue #2552837 by smustgrave, pwolanin, alexpott: XSS::filter and filter_xss...

Issue #2552837 by smustgrave, pwolanin, alexpott: XSS::filter and filter_xss can create malformed attributes when you would expect them to be stripped

(cherry picked from commit 4421948a)
parent 67aad0ff
No related branches found
No related tags found
5 merge requests!8506Draft: Issue #3456536 by ibrahim tameme,!5646Issue #3350972 by nod_: [random test failure]...,!5600Issue #3350972 by nod_: [random test failure]...,!5343Issue #3305066 by quietone, Rename RedirectLeadingSlashesSubscriber,!3603#ISSUE 3346218 Add a different message on edit comment
......@@ -265,6 +265,10 @@ protected static function attributes($attributes) {
break;
case 2:
// Once we've finished processing the attribute value continue to look
// for attributes.
$mode = 0;
$working = 1;
// Attribute value, a URL after href= for instance.
if (preg_match('/^"([^"]*)"(\s+|$)/', $attributes, $match)) {
$value = $skip_protocol_filtering ? $match[1] : UrlHelper::filterBadProtocol($match[1]);
......@@ -272,8 +276,6 @@ protected static function attributes($attributes) {
if (!$skip) {
$attributes_array[] = "$attribute_name=\"$value\"";
}
$working = 1;
$mode = 0;
$attributes = preg_replace('/^"[^"]*"(\s+|$)/', '', $attributes);
break;
}
......@@ -284,8 +286,6 @@ protected static function attributes($attributes) {
if (!$skip) {
$attributes_array[] = "$attribute_name='$value'";
}
$working = 1;
$mode = 0;
$attributes = preg_replace("/^'[^']*'(\s+|$)/", '', $attributes);
break;
}
......@@ -296,15 +296,13 @@ protected static function attributes($attributes) {
if (!$skip) {
$attributes_array[] = "$attribute_name=\"$value\"";
}
$working = 1;
$mode = 0;
$attributes = preg_replace("%^[^\s\"']+(\s+|$)%", '', $attributes);
}
break;
}
if ($working == 0) {
// Not well formed; remove and try again.
// Not well-formed; remove and try again.
$attributes = preg_replace('/
^
(
......
......@@ -107,7 +107,7 @@ public function providerTestFilterXss() {
// Default SRC tag by leaving it empty.
// @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Default_SRC_tag_by_leaving_it_empty
$data[] = ['<IMG SRC= onmouseover="alert(\'xxs\')">', '<IMG nmouseover="alert(&#039;xxs&#039;)">'];
$data[] = ['<IMG SRC= onmouseover="alert(\'xxs\')">', '<IMG>'];
// Default SRC tag by leaving it out entirely.
// @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Default_SRC_tag_by_leaving_it_out_entirely
......
......@@ -525,6 +525,24 @@ public function providerTestAttributes() {
'Link tag with numeric data attribute',
['a'],
],
[
'<img src= onmouseover="script(\'alert\');">',
'<img>',
'Image tag with malformed SRC',
['img'],
],
[
'Body"></iframe><img/src="x"/onerror="alert(document.domain)"/><"',
'Body"&gt;<img />&lt;"',
'Image tag with malformed SRC',
['img'],
],
[
'<img/src="x"/onerror="alert(document.domain)"/>',
'<img />',
'Image tag with malformed SRC',
['img'],
],
];
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment