Opened 8 months ago

Last modified 5 months ago

#34831 assigned Cleanup/optimization

Search in admin could allow issuing a query with many OR'd clauses

Reported by: Natalia Bidart Owned by: Yves Weissig
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As a logged in user, when performing searches in the django admin, a request with a lot of terms separated by spaces could cause the server to slow down because each term is OR'd, potentially building a bad query.

The root cause of this issue is a for loop in /django/contrib/admin/options.py:

    if search_fields and search_term:
            orm_lookups = [construct_search(str(search_field))
                          for search_field in search_fields]
            for bit in smart_split(search_term):
                if bit.startswith(('"', "'")):
                    bit = unescape_string_literal(bit)
                or_queries = [models.Q(**{orm_lookup: bit})
                              for orm_lookup in orm_lookups]
                queryset = queryset.filter(reduce(operator.or_, or_queries))
            use_distinct |= any(lookup_needs_distinct(self.opts, search_spec) for search_spec in orm_lookups)

The value of the string search_term comes from the frontend input search field and is then split with spaces using the smart_split.

Thanks Wenchao Li of Alibaba Group for the report.

Change History (5)

comment:1 by Natalia Bidart, 8 months ago

This was first reported as a security issue but the security team concluded that this can be discussed and fixed in the open, given that the user has to be already authenticated to issue the query (and the admin is considered secured).

But, this issue could happen accidentally from legitimate users, so the suggested fix is to limit the amount of terms that can OR'd in the resulting query.

comment:2 by Yves Weissig, 8 months ago

Happy to have a look at this Natalia Bidart.

Is limiting the amount of OR terms the agreed upon solution? Is there a discussion in the forum I'm missing?

comment:3 by Adam Johnson, 8 months ago

This issue was only discussed on the private security mailing list before being made into a ticket. I proposed the split limit and others agreed. I would guess the sensible limit is somewhere between 30 and 100 terms. Ideally we’d do some benchmarking before deciding on an exact number.

comment:4 by Yves Weissig, 8 months ago

Owner: changed from nobody to Yves Weissig
Status: newassigned

Cool, thanks Adam Johnson, I'll have a look.

comment:5 by Nicolas Lupien, 5 months ago

I did benchmark with MySQL 8.0 and this model:

class Posts(models.Model):
    title = models.CharField(max_length=100, db_index=True)
    slug = models.TextField(db_index=True)
    body = models.TextField(db_index=True)
    tags = models.CharField(max_length=100, db_index=True)
    created_at = models.DateTimeField(auto_now_add=True, db_index=True)

class PostAdmin(admin.ModelAdmin):
    search_fields = ["title", "slug", "tags", "body", "created_at"]
    list_display = ["title", "tags"]

10 terms: 673.32 ms
25 terms: 649.94 ms
50 terms: 615.97 ms
100 terms: 543.81 ms

Notice the query is actually improving in time when increasing the terms. I think this is ecause the query is only OR'd for each different field, for a single field the terms are actually AND'd, which limit the results a lot.

Here's a query with 2 terms (Hello World) :

SELECT ••• FROM `posts_posts` WHERE ((`posts_posts`.`title` LIKE '%Hello%' OR `posts_posts`.`slug` LIKE '%Hello%' OR `posts_posts`.`tags` LIKE '%Hello%' OR `posts_posts`.`body` LIKE '%Hello%' OR `posts_posts`.`created_at` LIKE '%Hello%') AND (`posts_posts`.`title` LIKE '%World%' OR `posts_posts`.`slug` LIKE '%World%' OR `posts_posts`.`tags` LIKE '%World%' OR `posts_posts`.`body` LIKE '%World%' OR `posts_posts`.`created_at` LIKE '%World%')) ORDER BY `posts_posts`.`id` DESC
Note: See TracTickets for help on using tickets.
Back to Top