🐛 [Bug]: Django (pongo2) integration vulnerability to XSS
bastianwegge opened this issue · 10 comments
Bug Description
Currently inside of pongo2 there's a setting that sets pongo2.SetAutoescape(false) which disables HTML sanitization, as far as I can see. This makes the gofiber/template/django integration unusable for my project.
How to Reproduce
Steps to reproduce the behavior:
- Create a basic project including gofiber + gofiber/template/django
- Create a variable with a string
<script>alert("ALERTAAAA");</script> - Print the variable into a template
- You have a potential XSS attack if the variable was created by user-input
Expected Behavior
I'd expect HTML sanitization to be active at all times. I'd also expect some documentation on how to explicitly deactivate sanitization for a non-user-input variable that is able to render valid HTML.
Template package Version
Django - v3.1.5
Code Snippet (optional)
No response
Checklist:
- I agree to follow Fiber's Code of Conduct.
- I have checked for existing issues that describe my problem prior to opening this one.
- I understand that improperly formatted bug reports may be closed without explanation.
We could add a config value for this, would that work for you @bastianwegge ?
@sixcolors I'd say yes. Is there a use case for having this disable?
@sixcolors I'd say yes. Is there a use case for having this disable?
Not sure. I’ll have to read pnogo2 docs. There are valid reasons to allow unsafe. But I don’t know why you would do it globally. Unless it doesn’t have a way to do it selectively.
@sixcolors It only works per template or globally. flosch/pongo2#312
We can add it as a config variable, and enable it by default
So docs are non existent for pongo2. But https://github.com/flosch/pongo2/blob/c84aecb5fa79a9c0feec284a7bf4f0536c6a6e99/template_tests/autoescape.tpl -> https://github.com/flosch/pongo2/blob/c84aecb5fa79a9c0feec284a7bf4f0536c6a6e99/template_tests/autoescape.tpl.out suggests it can be done in the template.
I would set it to true in the django.go and let them use disable in the template.
we could add a config option or not. But secure by default on templates would be my preference.
@sixcolors Agree! I will work on a PR
Hey @gaby, we went with something completely different because of the security issues. Rene suggested to open an issue so this wouldn't go unnoticed.
I plan on updating all the unit-tests today. The output is quite different that without escaping!