Cphalcon: Possible bug: SQL injection in tableExists(), viewExists()

[ghost]

Because table/column/index/etc names are never escaped (and the documentation does not whether it is up to the developer of framework to escape names), it is possible to inject an arbitrary SQL.

For example, for SQLite the following:

$dialect = new Phalcon\Db\Dialect\Sqlite();
echo $dialect->tableExists("test'; DROP TABLE test; SELECT * FROM sqlite_master WHERE '"), "\n";

produces

SELECT CASE WHEN COUNT(*) > 0 THEN 1 ELSE 0 END FROM sqlite_master WHERE type='table' AND tbl_name='test'; DROP TABLE test; SELECT * FROM sqlite_master WHERE ''

Whil I understand that these methods will almost never (hopefully) interact with untrusted data, it would be nice to make sure that there is no way for Little Bobby Tables out there.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ghost]

Or the documentation should be updated to tell that it is user's responsibility to pass sanitized data to those methods (I would, for example, assume that the framework takes care of these — and while for the PHP framework it may be easy to check the source to see what happens, this will be much more difficult for the C extension).

[phalcon]

The SQL dialect could return a string with positional placeholders, then execute fetchOne passing the parameters in an array.

[stale[bot]]

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

[niden]

This method was never designed to be used in areas where code can be injected into the tableExists or viewExists.

The only thing I can do right now is to use the escape method so that the schema and/or table names can be escaped.

The dialect is designed to return the SQL not execute it. It is up to the developer to make sure that the proper placeholders are used.

In 4.x we have plans to revisit the Db layer and offer additional checks for methods like the ones mentioned above.