yourlabs / django-autocomplete-light

A fresh approach to autocomplete implementations, specially for Django. Status: v4 alpha, v3 stable, v2 & v1 deprecated.
https://django-autocomplete-light.readthedocs.io
MIT License
1.8k stars 467 forks source link

XSS in select2.js #1223

Closed patrice-boulanger closed 3 years ago

patrice-boulanger commented 3 years ago

Hello,

django-autocomplete-light embeds select2.js v4..0.3 which has a XSS vulnerability:

https://github.com/select2/select2/issues/4587 https://snyk.io/vuln/npm:select2:20130108

Any possibility to upgrade ?

Many thanks!

jpic commented 3 years ago

Quoting the related issue:

the data that is retrieved remotely is not sanitized before displaying it, and this should be done to prevent XSS issues.

Despite that this is what Django developers believe, this is actually incorrect.

You must not trust user input, you must validate input properly, and not accept HTML everywhere and "hope" that your output will always be escaped.

You must not accept dangerous input to start with:

Input validation is performed to ensure only properly formed data is entering the workflow in an information system, preventing malformed data from persisting in the database and triggering malfunction of various downstream components. Input validation should happen as early as possible in the data flow, preferably as soon as the data is received from the external party.

Despite that Django accepts all kind of dangerous caracters by default, which is only /part/ of what makes Django "insecure by default". Anyway, be careful if you bring that up on django-developers mailing list because then you will be personally attacked by some core developers, who will literally tell you that it's perfectly normal to allow a user with a first name of <script>alert('hi')</script>, not far from smelly "it's more inclusive" arguments.

As such, you should not be affected by this security vulnerability, because you present validated and known safe input to select2 because all data that has entered your information system is validated and safe.

If unsure, then escape your content in the view is more curative than updating the script that uses data from the view anyway.

That said, I have nothing against making a release.

patrice-boulanger commented 3 years ago

If I totally agree with you about the responsability of developers to validate each user's inputs, let me give you a little bit more context about this requet.

We are using autocomplete-light module into a big project and it does perfectly the job but, during security audit, the select2.js library used by autocomplete is flagged as critical due to the XSS injection vulnerability.

To quote you, no developer should "hope" that the output will always be escaped, it means also that you can have a mistake into your code that doesn't correctly escape some specific HTML tags or attributes. We can also imagine that a new, never planned way to do XSS injection will be discovered tomorrow, defecting the security code that currently validate user's input.

Based on this, and even if 100% security doesn't exist, it seems relevant to me to provide security at all levels of the application.

Many thanks for your feedback.

Best,

jpic commented 3 years ago

Indeed, thanks for hearing me out, released in 3.8.2, please let me know if that works for you.

patrice-boulanger commented 3 years ago

I have updated the package, everything works fine, Thank you !

jpic commented 3 years ago

Thank you for your feedback, hopefully the CHANGELOG described how I failed to rebuild before last release so that's how I found how to not redo the mistake, but still wasn't very sure :joy:

Good luck for your audit!!!