mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
DDC-2756: QueryBuilder Sql Injection in order by #3448
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @doctrinebot on GitHub (Oct 23, 2013).
Originally assigned to: @Ocramius on GitHub.
Jira issue originally created by user bunny1985:
OrderBy($a, $b) allows to pass other string than ASC and DESC;
Thats' why it is possible to make sql Injection Attack. There is no Additional Validation in Query Builder.
For example pssing "DESC ; DROP DATABASE " as second argument allows ( depneding on dtabase setup) user to execute his own sql query.
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @ocramius:
The query builder is a string builder. It is not meant to allow user input. Whenever you have user input, you should use named or positional parameters and bind user input to those.
@doctrinebot commented on GitHub (Oct 23, 2013):
Issue was closed with resolution "Invalid"
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @beberlei:
[~bunny1985] Is this ORM or DBAL QueryBuilder related?
In the ORM this is detected in the Parser and not possible to do.
In DBAL, we allow arbitrary input for everything, that means security of SQL injcetion is burden of the user. This is because in an SQL QueryBuilder, you never know the context and cannot build a powerful API, that also prevents injections.
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by bunny1985:
ORM. most of the functions are in the end paremeters in query, or taking Int as an argument, but order by is just taken as it is without validation and there are only two valid options.
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @ocramius:
[~bunny1985] I agree there should be some strictness there, but you should never allow user input to get into DQL anyway.
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @beberlei:
[~bunny1985] Ok in this case this cannot lead to SQL injection. The DQL string is parsed and ASC/DESC are the only valid values there, leading to a parsing error. We could check the input for the 2 values, but it would lead to a different exception message (still producing the Doctrine\ORM\Query\QueryException type)
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @beberlei:
[~bunny1985] If you feel my statement is wrong, can you give a reproduce error that shows the problem?
@doctrinebot commented on GitHub (Oct 23, 2013):
Comment created by @beberlei:
For me anything without ASC/DESC throws an exception of the type:
[Syntax Error] line 0, col 70: Error: Expected end of string, got ';