SQL Injection is possible on orderBy

[TacticsJan]

When passing an orderby to the request there is an SQL Injection vulnerability.

For example /orderby/someTable.SOMEPROPERTY%20WAITFOR%20DELAY%20'0:0:10'-- will effectively delay the query.

I have fixed this in the symfony1 fork our company made ages ago. I will make a PR with my proposed fix for this issue here as well

[TacticsJan]

I can not push my branch because I do not have sufficient rights on this repository. So I will explain my proposed fix here.

BasePeer:772

if ($spacePos !== false) {
                    $direction = substr($columnName, $spacePos);
                    if (!in_array(trim($direction), array('ASC', 'DESC'))) {
                        $direction = '';
                    }
                    $columnName = substr($columnName, 0, $spacePos);
                }
                else {
                    $direction = '';
                }

[mentalstring]

Probably shouldn't go without saying that one should never trust user input, etc, etc. Still, thank you for sharing this with everyone.

ModelQuery::orderBy() already filters column names and sorting direction, so only Criteria::addAscendingOrderByColumn and Criteria::addAscendingOrderByColumn() seem to allow this.

I think the proposed fix isn't quite enough because at BasePeer::753 any further checks are skipped if the order by clause has a single ( anywhere it; I think this is to allow for things like ORDER BY RAND() expressions, so it would be easy to skip the fix. I'm not sure if checking for ; would be the way to go as there may be multiple ways to escape it or there may even be valid uses with it? Checking user input may be the most immediate solution and I'm not sure a fix would still be merged into Propel at this point given how stale it is.

Still, for anyone concerned with this, in MySQL it can be prevented by setting PDO::MYSQL_ATTR_MULTI_STATEMENTS to false which prevents multiple sql statements in a single query – a good security practice as it prevents this class of attacks, but may require some code changes if one happens to use multiple statements in a single query when doing custom PDO queries. The setting can be set on Propel's connection driver options.