Discussion: consider disabling PDO emulated query parser by default

[bobvandevijver]

I've been linked to a quite interesting article regarding possible SQL injection when using PDO: https://slcyber.io/assetnote-security-research-center/a-novel-technique-for-sql-injection-in-pdos-prepared-statements/. It describes a method that confuses the PDO emulated query parser into something that shouldn't happen with bound parameters, which in certain scenarios can be used to execute SQL injection.

This brings the question: why should we even want to use this emulation? From my understanding this is an artifact from the past, where the common consensus was that it would benefit performance as prepared statements would skip the query cache with older MySQL versions. But is that still relevant today?

Which is why I'd like to open a discussion: should dbal (or possibly orm) consider setting the PDO::ATTR_EMULATE_PREPARES to false by default?

Proof of concept with doctrine:

$address = $request->query->getString('q'); // http://somehost/some/page?q=?%00
dd($this
  ->getEntityManager()
  ->getConnection()
  ->executeStatement("SELECT * FROM address WHERE address1 LIKE '%$address%' AND city = ?", ['enschede'])
);

Which throws SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens, which is solved by adding ->option(PDO::ATTR_EMULATE_PREPARES, false) to the dbal driver configuration.

[morozov]

Why not propose this to PDO/PHP?

[bobvandevijver]

Great question, and I think I'm going to return with "why not both?". I do believe that it is easier to push this default change with Doctrine as then it doesn't depend on the version of PHP you're running (I do not think this default will be changed in PHP before PHP 9).

[morozov]

why not both?

Because if this is a real issue that needs to be fixed, fixing it in PHP will fix it everywhere. If someone affected cannot immediately upgrade, they can disable emulation manually.

There is no such thing as "both PHP and Doctrine DBAL". There is PDO and millions of applications and libraries that use PDO. Why do you want to fix it in "both", but not in the remaining libraries and applications?

Leave alone the fact that disabling emulation may noticeably impact performance with some databases (e.g. MySQL).

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.