Hi, On Tuesday, January 4, 2022 at 2:09:18 PM UTC+1 Sjoerd Job Postmus wrote:
> Unfortunately (at least for us), this breaks the case where dictsort was > used with a static argument that looked up a callable. A quick code search > showed that the pattern dictsort.*get​ ( > https://github.com/search?q=%22dictsort.*get%22&type=Code) is less used > than I would expect it to be used, but used nonetheless. > I expected as much; literally every security fix nowadays will break some code. Sorry about that. You can change it to a property or write your own filter I guess. On the other hand, searching publicly viewable code for cases where > dictsort gets a dynamic value yields very little results: > https://grep.app/search?q=dictsort%3A%5B%5E%22%270-9%5D®exp=true&case=true, > > only one which I can recognize as a Django template: > https://github.com/crodas/Haanga/blob/develop/tests/assert_templates/regroup.tpl#L3 > . > Yes, that said there is no easy way for a template filter to determine whether it is a variable or a literal (I'd say it is simply impossible). Our first approach would have been to limit only those values provided via variables… I do not think that is possible in a nice way though and probably to much for a security patch. Since the previous behaviour of allowing callables was in place already in > 2005, (I could find > https://github.com/django/django/commit/ed114e15106192b22ebb78ef5bf5bce72b419d13#diff-e05e2e8efbf1fa5eea1ffee16cc8b740cba7b1bff746b2e55cebf968a0983f2cR192, > > where the filter is introduced), I would argue that even though it may not > have been explicitly documented that this syntax allows callables, I don't > think it's far fetched to consider it to always have supported. > I would argue that the Django team always said that the documentation is the public API. Everything else works by luck. I even went as far as supporting lookups on objects (ie a list of objects as opposed to a list of dicts) because I assumed that people would use that often (and the docs clearly say this filter is for dicts). However, before creating a ticket, I was wondering what the sentiment of > django-developers is here. My own sentiments are summarized by what's also > mentioned in the announcement blogpost. > >From a security PoV I think this is not going to fly. The main issue here is that dictsort should perform a rather limited subset of normal template variable resolution -- if we were going to support callables again we'd have to also support `alter_data` etc (which is forbidden in templates) and then we are basically back to what the previous code did. We opted for the most limited subset possible while allowing a relatively wide range of code to keep working, I don't think adding more features to that filter is feasible. > > As a reminder, all untrusted user input should be validated before use. > > As an example, even with the change, {% for user in > users|dictsort:"password" %} would still be supported, which is still be > counted as potential information disclosure. > There is a massive difference though. Being able to sort by every index inside the password means that you need to control far less password hashes than in your example to get a useful result. To be honest I do not think it is very realistic to use that attack on the password; but it might be usable for shorter api tokens or so that you can also view in plaintext. I hope that helps. Cheers, Florian -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6ee7a4a3-f0b3-451e-a250-71b2a31b4722n%40googlegroups.com.
