On Wed, Mar 26, 2014 at 10:30 AM, Nuria Ruiz nuria@wikimedia.org wrote:
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.
Urls in the template engine need to be handled on their own, sure. But what template engine does not work in this fashion? There are three separate "entities" you normally deal with when doing replacement: translations, urls and plain attributes.
When looking at a typical web page, you need several escaping strategies. OWASP roughly groups them into html body, plain attributes, URL context, Javascript context, CSS context. My point was that you need several MakeWhateverSafe functions, and have to use them in the right context. So that is a long way of saying I disagree with you when you said that this could be automated without some process having knowledge of the html context and verifying the right escaping is being applied.
$html = Html::element( 'div', array( 'class' => $anything ), $anythingElse
I see. Sorry but where I disagree is that the "quote me this replacement" is a lawful case for the template engine. The line above is doing a lot more than purely templating and on my opinion it does little to separate data and markup. Which is the very point of having a template engine.
But if you consider that one a lawful use case, you are right. The example I provided does not help you.
On Wed, Mar 26, 2014 at 6:15 PM, Chris Steipp csteipp@wikimedia.org wrote:
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/
]
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l