laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

Blade XSS prevention , contextual escaping #2339

Open ivanomatteo opened 4 years ago

ivanomatteo commented 4 years ago

As far I know in Blade there are no special directive to escape data. for example: https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-2-attribute-encode-before-inserting-untrusted-data-into-html-common-attributes

in fact (for example) missing attribute quotes, with default Blade escaping could end in a XSS:

<div class={{ 'border rounded onclick=alert(1)' }}>
       hello!!!
 </div>

I found this old repository: https://github.com/laravelgems/blade-escape

I wonder why such simple directives are not included in Blade.

For Example TWIG support it: https://twig.symfony.com/doc/3.x/filters/escape.html

and LATTE do it automatically https://latte.nette.org/en/security#toc-context-aware-escaping

Automatic context aware escaping would be perfect, but i guess that would be a little more complicated.

What do you think about?

ali-alharthi commented 4 years ago

Your example is correct and it could result to XSS. However, why would a programmer pass or allow a user to pass something in the class (your example). I feel like it is quite a dump thing to-do. I believe as a developer it is your job to make sure that user's inputs are validated/sanitized in every case. Also, for example: Blade has {!! $somthing !!} that allow html/no-escape and that will results in XSS, does that mean Blade is bad? No, it is on the developer who used it incorrectly. I personally use {!! !!} for example, when passing certain errors (flash/session) to add line breaks (<br>) in them. But, I would never use it on a user input even if I validated that input.

BTW here is a fix:

<div class='{{ 'border rounded onclick=alert(1)' }}'>
    hello!!!
</div>

or double-quotation around the class attribute 👍🏼

ivanomatteo commented 4 years ago

You are right, using with quote, blade escaping for attribute is safe. But one the purposes of a template engine, is prevent programmer's errors: it's easy to forget quotes.

Moreover attribute escaping is not the only case, where a special escaping is useful.

Inside a style tag for example, htmlspecialchars is not the right escaping.

ali-alharthi commented 4 years ago

But one the purposes of a template engine, is prevent programmer's errors: it's easy to forget quotes.

That is how HTML/CSS work, if you have css classes it should be as a value for class using no quote at all won't work even with regular HTML! Blade expect you to pass in variables - Controller: $data = 'border rounded onclick=alert(1)'; then on Blade: {{ $data }} right? How would you add that to a class? Use single/double quote on the class: <div class='{{ $data }}' >.