Opened 16 years ago
Closed 16 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 |
Pull Requests: | How to create a pull request | ||
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.
Change History (6)
by , 16 years ago
Attachment: | 10160.diff added |
---|
comment:1 by , 16 years ago
Has patch: | set |
---|
by , 16 years ago
Attachment: | 10160-2.diff added |
---|
comment:2 by , 16 years ago
comment:3 by , 16 years ago
Yeah that's defintely an improvement F('field') - 'string' shouldn't turn 'string' into a fieldname automatically.
comment:4 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Note:
See TracTickets
for help on using tickets.
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 usingF('surplus')
instead, which is better anyway since it's explicit.