fix: disable PDO emulated prepared statements (issue #1859)

[kayjoosten]

Set PDO::ATTR_EMULATE_PREPARES to false on all DBAL connections.
By default PDO uses its own C-level parser to interpolate parameters,
which is incomplete and vulnerable to SQL injection via multi-byte
charset encoding tricks. Setting this to false forces native prepared
statements on the MariaDB server side, where parameter binding is
handled correctly.

issue #1859

[kayjoosten]

@johanib @baszoetekouw lets discuss this in the refinement. Because we disabled the pdo prepared statements you also disable named param bindings. MariaDB does not support it only positional bindings. Im not sure if we want to actually disable it. As for the risks pdo gives the utf_8 would take that away. As backslashes are not allowed in utf_8. I have a list here that ai assabled with what its worth for.

  Why UTF-8 reduces the risk (but doesn't eliminate it)

  The classic SQL injection via emulated prepares exploits multi-byte character encoding tricks. The attack works like this:

  With emulated prepares, PDO escapes your value by prepending a backslash to dangerous characters. In some multi-byte encodings (like GBK, BIG5, Shift-JIS), a valid character's second byte can be 0x5C — which
  is ASCII \. PDO sees it as "escaped" but the DB sees it as part of a valid multi-byte character, leaving the following ' unescaped → injection.

  UTF-8 doesn't have this problem because it was specifically designed so no continuation byte (0x80–0xBF) can be confused with ASCII characters (0x00–0x7F). So 0x5C (\) can never appear as a continuation byte
  in valid UTF-8.

  BUT — I'd still recommend disabling emulation, here's why:

  1. The parser is incomplete regardless of charset PHP's PDO emulation parser is not a full SQL parser. It has known edge cases with:

   - Comments (-- comment or /*
    */ containing ? or :name)
   - String literals inside strings
   - Operator sequences that look like parameters

  2. Your connection charset ≠ your data If any legacy data was stored with a different charset, or a client connects with SET NAMES gbk, the assumption breaks.

  3. Native prepares enforce type safety at the protocol level The DB receives the SQL and parameters completely separately — there is literally no way for parameter data to influence query structure. With
  emulation, it's still string concatenation under the hood, just with escaping.

[johanib]

Though PR...

Found this interesting: https://phpdelusions.net/pdo#emulation

I think these changes are 👍 and should be good.
Also tested the disabling of this feature in a random project and nothing broke. So only if you have custom queries with named params, something can break.

One thing to note, which might be relevant to @baszoetekouw :
Before this change, PDO would send the query to the database in one go.
After this change, it is potentially sent to the db in two parts: First the query, then the parameters, because we use db native prepared statements.

This is not an issue when the latency between webserver and db server is very low (<1ms). But if there are scenario's where the db is remote, this can cause delays, as each query can potentially need two instead of one request to the database.
This cost applies to all queries using parameters, because doctrine basically does this:

if (count($params) > 0) {
    $stmt = $connection->prepare($sql);   // ← COM_STMT_PREPARE
    $this->bindParameters($stmt, $params, $types);
    $result = $stmt->execute();           // ← COM_STMT_EXECUTE
} else {
    $result = $connection->query($sql);   // ← single round trip, no params
}

mysqlnd does cache the prepared statements on a connection/process level, so the actual performance impact might be minimal.

[johanib]

Suggested change

	$query->where('entity_id = :entity_id');
	$query->where('entity_id = :entity_id_where')
	->setParameter('entity_id_where', $role->entityId, Types::STRING);

I think this goes slightly beyond the scope of your ticket, but related. This :entity_id is not set in this file. But it flows from the entity itself, via the loop above.
It makes me wonder why this where is needed in the first place 🤔
But to be safe, might as well bind it explicitly while we are at it.

$query->where('entity_id = :entity_id');
$this->addDiscriminatorQuery($query, $metadata);
$query->executeStatement();
The WHERE entity_id = :entity_id clause references the named parameter :entity_id. This value is never explicitly bound via $query->setParameter('entity_id', ...) in the new code.

The reason it works in practice is accidental: entity_id is an ORM-mapped column (#[ORM\Column(name: 'entity_id')] in AbstractRole), so it flows into normalizeData(), and the foreach loop in updateRole() calls $query->set('entity_id', ':entity_id') and $query->setParameter('entity_id', ...) as part of the SET clause. The parameter binding therefore exists as a side-effect of setting the column value, not because it was explicitly bound for the WHERE clause.

This is fragile for two reasons:
1. If entity_id is ever removed from ORM field mappings (e.g., made a join column or renamed), the WHERE clause silently loses its binding. Depending on the DBAL/driver version, this could mean an unbound placeholder in the query, a driver error, or — worst case — an unfiltered UPDATE that touches every row in the table.
 
2. It introduces an implicit coupling: the WHERE target and the SET value share one binding. An UPDATE ... SET entity_id = :entity_id WHERE entity_id = :entity_id is only correct when updating with the same entity_id, which is true here because the entity is being updated in-place. But future maintainers have no way of knowing this without deep investigation.
 
3. Recommended fix — make the intent explicit:private function updateRole(AbstractRole $role, ClassMetadata $metadata): void
{
    $query = $this->connection->createQueryBuilder()->update(self::ROLES_TABLE_NAME);
    foreach ($this->normalizeData($role, $metadata) as $columnName => $fieldData) {
        $query->set($columnName, ':' . $columnName)
            ->setParameter($columnName, $fieldData[self::FIELD_VALUE], $fieldData[self::FIELD_TYPE]);
    }
    // Explicitly bind the WHERE parameter, independent of the SET clause loop above.
    $query->where('entity_id = :entity_id_where')
        ->setParameter('entity_id_where', $role->entityId, Types::STRING);
    $this->addDiscriminatorQuery($query, $metadata);
    $query->executeStatement();
}
Using a distinct parameter name (entity_id_where) removes the implicit dual-binding and makes the intent unambiguous.

[johanib]

This runs DELETE FROM consent without any predicate, wiping all consent rows in the test database. This is a test pollution risk.

Suggested change

	$qb->delete('consent')->executeStatement();
	private function clearConsentFixtures(): void
	{
	foreach (self::TEST_USER_IDS as $userId) {
	$this->connection->createQueryBuilder()
	->delete('consent')
	->where('hashed_user_id = :hashed_user_id')
	->setParameter('hashed_user_id', sha1($userId))
	->executeStatement();
	}
	}