As some of you might have noticed the initial patch to prevent data leakage 
in
contrib.admin via query string manipulation[1] introduced many
regressions[2][3][4] that resulted in two security releases so far.

The actual implementations does four checks to ensure a field is allowed to
be referenced:

1. It makes sure the field exists on the model[5]. This check have not
   introduced a regression so far.

2. It allows reference to the primary key of models that define a least one 
many
   to many relationship even if the foreign model is not registered to the
   admin[6]. This is an extra branching added in order to solve #23604[4].

3. It only allows reference from models registered directly or through
   inlines[7]. The inline case was added in order to partially solve 
#23431[3].

4. It only allows fields referenced by at least one of the collected 
registered
   model fields[8]. This part was adjusted to consider hidden 
relationships[3],
   many to many fields and model inheritance[2].

Recently another regressions surfaced[9] and Julien Phalip provided an 
initial
patch to fix it[10].

The patch proposes to always allow reference to the primary key field and
fallback to the related object detection in the other cases. It doesn't
introduce a security issue because primary key existence can already be 
guessed
from the model admin change URL. If primary key references would have been
allowed from the begining none of the regressions would have shown up.

After further analysis I'm pretty confident this is the correct solution 
but I'd
like to request feedback in order to avoid further regressions.

Thanks for your time,
Simon

[1] 
https://github.com/django/django/commit/53ff0969822ac2248a89ccb6fef1088212dc800d

[2] https://code.djangoproject.com/ticket/23329
[3] https://code.djangoproject.com/ticket/23431
[4] https://code.djangoproject.com/ticket/23604

[5] 
https://github.com/django/django/blob/de912495ab1c3b3e05e79068861ab6b3b5f5c32e/django/contrib/admin/options.py#L450-L455
[6] 
https://github.com/django/django/blob/de912495ab1c3b3e05e79068861ab6b3b5f5c32e/django/contrib/admin/options.py#L457-L460
[7] 
https://github.com/django/django/blob/de912495ab1c3b3e05e79068861ab6b3b5f5c32e/django/contrib/admin/options.py#L462-L468
[8] 
https://github.com/django/django/blob/de912495ab1c3b3e05e79068861ab6b3b5f5c32e/django/contrib/admin/options.py#L470-L475

[9] https://code.djangoproject.com/ticket/23754
[10] https://github.com/django/django/pull/3568/files

-- 
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 [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9907cd98-5271-47b7-94d5-1244aa55227e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to