-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Use psalm literal-string type, to address Injection Vulnerabilities #8933
Description
Feature Request
| Q | A |
|---|---|
| New Feature | yes |
| RFC | yes |
| BC Break | maybe |
Summary
Psalm 4.8 introduced a new literal-string type, which addresses the main source of Injection Vulnerabilities - developers incorrectly including user-input in sensitive strings, before they are provided to Doctrine, e.g.
$result = $qb
->select('u')
->from('User', 'u')
->where('u.id = ' . $_GET['id']); // INSECUREliteral-string works by distinguishing strings from a trusted developer, from strings that may be attacker controlled.
Considering QueryBuilder::add() already uses string|object|array for $dqlPart, the same could be used for where() $predicates.
However, by using literal-string|object|array for where($predicates), the literal-string type will check the developer wrote that string, and gets them to use setParameter() for user-input (as they should).
The literal-string type could be used in a few other locations as well (especially with the Injection risks that come with DQL); but I'd like to start the discussion with where().
The only issue I can see is Connection::quoteIdentifier(), for those rare times when user-input is used for table/field/etc names. Because it can return a non literal-string value, it cannot be concatenated into $predicates; so maybe there should be a QueryBuilder::setIdentifier() to ensure these values are always quoted correctly, something like:
$result = $qb
->select('u')
->from('User', 'u')
->where('{my_field} = :my_value')
->setIdentifier('my_field', $_GET['field'])
->setParameter('my_value', $_GET['id']);