SQL injection possible with limit() on MySQL #1463
Comments
|
Care to provide a PR? |
|
Happy to! Coming soon. |
|
@mpetrovich: good spot. However, food for thought - it is usual practice to notify a project of security problems privately, rather than making it public in the first instance. |
|
Good point, pardon my rush to post it here. Maintainers: Please delete these issues in the meantime if you’d like—I’m working on a pull request tonight to address this issue. |
When constructing a MySQL LIMIT clause, values for the offset and limit are coerced to integers. This prevents arbitrary SQL from being injected via the limit parameter. Example:
UserQuery::create()->limit('1;DROP TABLE users')->find();
Previously, this would have injected `DROP TABLE users` into the generated SQL. Now, the limit value would be coerced to the integer `1`.
Fixes propelorm#1463
When constructing a MySQL LIMIT clause, values for the offset and limit are coerced to integers. This prevents arbitrary SQL from being injected via a query limit. Example:
UserQuery::create()->limit('1;DROP TABLE users')->find();
Previously, this would have injected `DROP TABLE users` into the generated SQL. Now, the limit value would be coerced to the integer `1`.
Fixes propelorm#1463
|
PR with a fix: #1464 |
When constructing a MySQL LIMIT clause, values for the offset and limit are coerced to integers. This prevents arbitrary SQL from being injected via a query limit. Example:
UserQuery::create()->limit('1;DROP TABLE users')->find();
Previously, this would have injected `DROP TABLE users` into the generated SQL. Now, the limit value would be coerced to the integer `1`.
Fixes propelorm#1463
When constructing a MySQL LIMIT clause, values for the offset and limit are coerced to integers. This prevents arbitrary SQL from being injected via a query limit. Example:
UserQuery::create()->limit('1;DROP TABLE users')->find();
Previously, this would have injected `DROP TABLE users` into the generated SQL. Now, the limit value would be coerced to the integer `1`.
Fixes propelorm#1463
|
@marcj: I think this should this get an alert for the relevant versions on the Symfony security checker. Do you agree that all prior versions were vulnerable? I suspect the PR to that service needs to specify what versions are affected, and thus it needs to be accurate. Does this need a CVE reference? I don't know how to go about getting that assigned. |
) When constructing a MySQL LIMIT clause, values for the offset and limit are coerced to integers. This prevents arbitrary SQL from being injected via a query limit. Example: UserQuery::create()->limit('1;DROP TABLE users')->find(); Previously, this would have injected `DROP TABLE users` into the generated SQL. Now, the limit value would be coerced to the integer `1`. Fixes #1463
|
Partly to answer my own question, I see you have a bug report for each of the major versions. Thanks. I have prepared a suggested change and will submit that as a PR to the sec database tomorrow (Sunday 18th Feb). If you are around and are able to double-check the contents, that'd be great. In particular, I don't know if Presently for Propel1 I have marked everything including the current release as vulnerable. If a new release is made, ping me and I will push a new change. |
I believe so. According to the Composer docs, any alpha version like
I've made pull requests [1] [2] to fix the same vulnerabilities in Propel v1 and v3, so you should be able to restrict the vulnerability version range to everything up to the current releases. |
|
Marvellous, thanks @mpetrovich. I've set the Propel1 constraints to I'll raise a PR just now and will liaise with maintainers to get this merged into the advisories db. Once that is done I'll build a demo Propel1 and Propel2 project to make sure the warnings come up on the latest releases. That will double-check the alpha thing too. |
The
limit()query method is susceptible to catastrophic SQL injection with MySQL.For example, given a model
Userfor a tableusers:This will drop the
userstable!The cause appears to be a lack of integer casting of the limit input in either
Propel\Runtime\ActiveQuery\Criteria::setLimit()or inPropel\Runtime\Adapter\Pdo\MysqlAdapter::applyLimit(). The code comments there seem to imply that casting was avoided due to overflow issues with 32-bit integers.This is surprising behavior since one of the primary purposes of an ORM is to prevent basic SQL injection.
This affects all versions of Propel: 1.x, 2.x, and 3.
Related:
The text was updated successfully, but these errors were encountered: