Opened 12 years ago

Closed 12 years ago

#10160 closed (fixed)

F expression literals should use bind parameters

Reported by: Ian Kelly Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


To prevent SQL injection attacks, Python values used within F() expressions should be processed as bind parameters rather than be inserted directly into the SQL.

Attachments (2)

10160.diff (1.3 KB) - added by Ian Kelly 12 years ago.
10160-2.diff (1.4 KB) - added by Ian Kelly 12 years ago.

Download all attachments as: .zip

Change History (6)

Changed 12 years ago by Ian Kelly

Attachment: 10160.diff added

comment:1 Changed 12 years ago by Ian Kelly

Has patch: set

Changed 12 years ago by Ian Kelly

Attachment: 10160-2.diff added

comment:2 Changed 12 years ago by Ian Kelly

The first patch doesn't quite work. It successfully passes non-string types as bind parameters, but string values were still just getting processed by quote_name, and string values are the most dangerous.

The new patch fixes this by removing the attempt to process the value with quote_name entirely. This introduces a minor backward incompatibility, since it's no longer possible to do things like .filter(amount__lt=F('max_amount')-'surplus') and have 'surplus' automatically be included as a field name. But this can still be accomplished by using F('surplus') instead, which is better anyway since it's explicit.

comment:3 Changed 12 years ago by Alex Gaynor

Yeah that's defintely an improvement F('field') - 'string' shouldn't turn 'string' into a fieldname automatically.

comment:4 Changed 12 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [9820]) Fixed #10160 -- Modified evaluation of F() expressions to protect against potential SQL injection attacks. Thanks to Ian Kelly for the suggestion and patch.

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