Skip to content

Commit

Permalink
Html: workaround for innerHTML mXSS vulnerability [Closes nette/nette…
Browse files Browse the repository at this point in the history
…#1496]

IE8 for code `<div attr="``foo=bar">` produces invalid innerHTML `<div attr=``foo=bar>`. Adding a space at the end of the attribute forces IE to put quotes around the attribute.

More info:
http://www.nds.rub.de/research/publications/mXSS-Attacks/
http://www.slideshare.net/x00mario/the-innerhtml-apocalypse
  • Loading branch information
dg committed May 24, 2014
1 parent 2efcd6e commit b81d18e
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/Utils/Html.php
Expand Up @@ -523,7 +523,10 @@ public function attributes()
$v = Json::encode($v);
}
$q = strpos($v, '"') === FALSE ? '"' : "'";
$s .= ' data-' . $k . '=' . $q . str_replace(array('&', $q), array('&amp;', $q === '"' ? '&quot;' : '&#39;'), $v) . $q;
$s .= ' data-' . $k . '='
. $q . str_replace(array('&', $q), array('&amp;', $q === '"' ? '&quot;' : '&#39;'), $v)
. (strpos($v, '`') === FALSE ? '' : ' ')
. $q;
}
}
continue;
Expand Down Expand Up @@ -551,7 +554,10 @@ public function attributes()
}

$q = strpos($value, '"') === FALSE ? '"' : "'";
$s .= ' ' . $key . '=' . $q . str_replace(array('&', $q), array('&amp;', $q === '"' ? '&quot;' : '&#39;'), $value) . $q;
$s .= ' ' . $key . '='
. $q . str_replace(array('&', $q), array('&amp;', $q === '"' ? '&quot;' : '&#39;'), $value)
. (strpos($value, '`') === FALSE ? '' : ' ')
. $q;
}

$s = str_replace('@', '&#64;', $s);
Expand Down
1 change: 1 addition & 0 deletions tests/Utils/Html.basic.phpt
Expand Up @@ -66,6 +66,7 @@ test(function() {

test(function() { // attributes escaping
Assert::same( '<a one=\'"\' two="\'" three="<>" four="&amp;amp;"></a>', (string) Html::el('a')->one('"')->two("'")->three('<>')->four('&amp;') );
Assert::same( '<a one="``xx\' " two=\'``x" \'></a>' , (string) Html::el('a')->one("``xx'")->two('``x"') ); // mXSS
});


Expand Down
3 changes: 2 additions & 1 deletion tests/Utils/Html.data.phpt
Expand Up @@ -19,8 +19,9 @@ test(function() { // deprecated
$el->data['d'] = '';
$el->data['e'] = 'two';
$el->{'data-x'} = 'x';
$el->data['mxss'] = '``two';

Assert::same( '<div data-a="one" data-d="" data-e="two" data-x="x"></div>', (string) $el );
Assert::same( '<div data-a="one" data-d="" data-e="two" data-mxss="``two " data-x="x"></div>', (string) $el );
});


Expand Down

2 comments on commit b81d18e

@mishak87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind-blown!

@fprochazka
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thank you! :)

Please sign in to comment.