[Django] #34831: Search in admin could allow issuing a query with many OR'd clauses

[Django]

#34831: Search in admin could allow issuing a query with many OR'd clauses
------------------------------------------------+------------------------
Reporter: Natalia Bidart | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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`:

{{{#!python
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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34831>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

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

--------------------------------------+------------------------------------

Reporter: Natalia Bidart | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------

Comment (by Natalia Bidart):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34831#comment:1>

[Django]

#34831: Search in admin could allow issuing a query with many OR'd clauses
--------------------------------------+------------------------------------
Reporter: Natalia Bidart | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Yves Weissig):

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?

--
Ticket URL: <https://code.djangoproject.com/ticket/34831#comment:2>

[Django]

#34831: Search in admin could allow issuing a query with many OR'd clauses
--------------------------------------+------------------------------------
Reporter: Natalia Bidart | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Adam Johnson):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34831#comment:3>

[Django]

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

-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Yves
Type: | Weissig
Cleanup/optimization | Status: assigned

Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Yves Weissig):

* owner: nobody => Yves Weissig
* status: new => assigned

Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/34831#comment:4>

[Django]

#34831: Search in admin could allow issuing a query with many OR'd clauses
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Yves
Type: | Weissig
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Nicolas Lupien):

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
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/34831#comment:5>