🐛 [Bug]: Django (pongo2) integration vulnerability to XSS

[bastianwegge]

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:

1. Create a basic project including gofiber + gofiber/template/django
2. Create a variable with a string <script>alert("ALERTAAAA");</script>
3. Print the variable into a template
4. 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.

[gaby]

We could add a config value for this, would that work for you @bastianwegge ?

[sixcolors]

@gaby shouldn't we set it to true?

template/django/django.go

Line 104 in 9dd46c8

pongo2.SetAutoescape(false)

@gofiber/maintainers This seems like a security vulnerability to me.

[gaby]

@sixcolors I'd say yes. Is there a use case for having this disable?

[sixcolors]

@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.

[gaby]

@sixcolors It only works per template or globally. flosch/pongo2#312

We can add it as a config variable, and enable it by default

Rel flosch/pongo2#261

[sixcolors]

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.

[gaby]

@sixcolors Agree! I will work on a PR

[bastianwegge]

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.

[gaby]

I plan on updating all the unit-tests today. The output is quite different that without escaping!

[sixcolors]

Could be in the way the template is passed in. I’ll take a look when I get off work. On Jan 8, 2024, at 10:27 AM, Juan Calderon-Perez ***@***.***> wrote: I plan on updating all the unit-tests today. The output is quite different that without escaping! —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>