On Wed, Mar 26, 2014 at 9:44 AM, Daniel Friesen daniel@nadir-seen-fire.comwrote:
On 2014-03-26, 9:32 AM, Nuria Ruiz wrote:
The issue is that they apply the same escaping, regardless of the html context. So, in Twig and mustache, <div class={{something}}></div>
is
vulnerable, if something is set to "1234 onClick=doSomething()".
Right, the engine would render:
<div class=1234 onClick=doSomething()> </div>
because it only escapes HTML by default. Now, note that the problem can be fixed with <div class={{makeStringSafe something}}>
Where "makestringSafe" is a function defined by us and executed there
that
escapes to our liking.
How does a custom function jammed into the middle of a Mustache template fix the issue when the issue is not that foo={{something}} doesn't escape, but is that quoting is needed instead of escaping, and Mustache isn't context sensitive so neither Mustache or a custom function know that foo={{something}} is an attribute value in need of quoting?
Exactly. Additionally, how you escape a plain parameter like class vs. an href vs. a parameter that is inserted into a url vs. an id attribute are all different escaping strategies. So there would be many different "makeXXXXStringSafe" and probably "quoteAndMakeXXXXStringSafe" functions, and code review would have to make sure the right one was being used in the right place. Which means someone who is familiar with all of the xss techniques would need to code review almost all the templates.
For comparison, using our current html templating (as much as it sucks):
$html = Html::element( 'div', array( 'class' => $anything ), $anythingElse );
The developer doesn't need to have any knowledge of what escaping needs to apply to the class attribute vs the text.
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://danielfriesen.name/]