Opened 8 years ago

Closed 5 years ago

#5023 closed (wontfix)

Tighten security around generic views

Reported by: Chui Tey Owned by: nobody
Component: Generic views Version: master
Severity: Keywords:
Cc: ivanov.maxim@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Forms with edit_inline fields include the PK in hidden inputs.
However, this should not be trusted.
If PK was specified and no related object was retrieved, then reset the PK to NULL. A new row will be created instead of modifying an existing row.

You should be able to test this by adding a hidden input to the top of mymodel_form.html to an unpatched system. This will result in a child row being hijacked from another parent.

Index: db/models/
--- db/models/   (revision 5773)
+++ db/models/   (working copy)
@@ -168,7 +168,10 @@
                                 old_rel_obj = getattr(self.original_object, related.get_accessor_name()).get(**{'%s__exact' % rel_new_data[][0]})
                             except ObjectDoesNotExist:
-                                pass
+                                # Security: PK should not be trusted as it comes from the web.
+                                #   if PK is in the POSTed data, and an object was not fetched from the database
+                                #   then reset it's PK to NULL
+                                rel_new_data[][0] = u''

                     for f in related.opts.fields:
                         if f.core and not isinstance(f, FileField) and f.get_manipulator_new_data(rel_new_data, rel=True) in (None, ''):

Change History (3)

comment:1 Changed 8 years ago by Simon G. <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 years ago by redbaron

  • Cc ivanov.maxim@… added

comment:3 Changed 5 years ago by Alex

  • Resolution set to wontfix
  • Status changed from new to closed

Involves manipulators, no way it's still valid.

Note: See TracTickets for help on using tickets.
Back to Top