Opened 18 years ago

Closed 18 years ago

#10160 closed (fixed)

F expression literals should use bind parameters

Reported by: Erin Kelly Owned by: nobody
Component: Database layer (models, ORM) Version: dev
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

Description

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 Erin Kelly 18 years ago.
10160-2.diff​ (1.4 KB ) - added by Erin Kelly 18 years ago.

Download all attachments as: .zip

Change History (6)

by Erin Kelly, 18 years ago

Attachment: 10160.diff​ added

comment:1 by Erin Kelly, 18 years ago

Has patch: set

by Erin Kelly, 18 years ago

Attachment: 10160-2.diff​ added

comment:2 by Erin Kelly, 18 years ago

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 by Alex Gaynor, 18 years ago

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

comment:4 by Russell Keith-Magee, 18 years ago

Resolution: → fixed
Status: new → closed

(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