Opened 8 years ago

Closed 7 years ago

#3145 closed defect (invalid)

edit_inline Manipulator processor allows "stealing" of related objects

Reported by: jag@… Owned by: nobody
Component: Core (Other) Version:
Severity: normal Keywords: security
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

This may be the desired behavior, but I'd probably consider it a security bug. I'll let you decide, and if you agree, I'll come up with a patch for it...

Let's say I have a shopping cart system. So I have two models:

 class ShoppingCart(models.Model):
   # ... snip ...
 
 class ShoppingCartItem(models.Model):
   shoppingcart = models.ForeignKey(ShoppingCart, edit_inline=models.STACKED)
   product = models.ForeignKey(Product, core=True)
   quantity = models.IntegerField()
   # ... snip ...

Then I have a view using django.views.generic.create_update.update_object and a template that contains:

 
 <form action="." method="post">
  <input type="text" name="shoppingcartitem.0.id" value="3" />
  <input type="text" name="shoppingcartitem.0.product" value="3" />
  <input type="text" name="shoppingcartitem.0.quantity" value="3" />
  <input type="submit" />
 </form>
 

If Alice creates a shopping cart with one item, 3 "Product 3"'s as above, and then Mallory comes in to her own shopping cart (not actually accessing the same ShoppingCart object as Alice but a distinct on of her own) and posts to this view with the values: shoppingcartitem.0.id=3&shoppingcartitem.0.product=3&shoppingcartitem.0.quantity=100, the item will be removed from Alice's cart and placed into Mallory's cart with a quantity of 100.

I would expect that Django would beef about either a) letting Mallory reassign the ForeignKey value for ShoppingCartItem(pk=3) or b) letting Mallory alter the value of ShoppingCartItem's not related to the ShoppingCart object she's viewing. Neither occurs.

Thoughts?

-jag

Change History (4)

comment:1 Changed 8 years ago by Gary Wilson <gary.wilson@…>

  • Triage Stage changed from Unreviewed to Accepted

seems like a nasty bug to me.

comment:2 Changed 8 years ago by Fast exchanger

  • Cc None added
  • Component changed from Core framework to Documentation
  • Keywords None added
  • Owner changed from adrian to jacob
  • Summary changed from edit_inline Manipulator processor allows "stealing" of related objects to Fast exchanger
  • Triage Stage changed from Accepted to Ready for checkin
  • Version set to unicode

Cool website! Your web site is helpful, All the best!

comment:3 Changed 8 years ago by SmileyChris

  • Cc None removed
  • Component changed from Documentation to Core framework
  • Keywords security added; None removed
  • Needs tests set
  • Owner changed from jacob to adrian
  • Summary changed from Fast exchanger to edit_inline Manipulator processor allows "stealing" of related objects
  • Triage Stage changed from Ready for checkin to Accepted
  • Version unicode deleted

comment:4 Changed 7 years ago by ubernostrum

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

A foreign key is just a foreign key; it has no inherent "meaning". If the Bar model has a foreign key to Foo, that could mean "User A owns this instance and changing it is a security breach", but it could also mean "User A is working with this instance right now but User B can take it over" or it could "mean" something else entirely. It's up to you -- the developer of your specific application -- to decide what the foreign key "means" and implement the appropriate logic.

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