Use psalm literal-string type, to address Injection Vulnerabilities #6809

Open
opened 2026-01-22 15:39:13 +01:00 by admin · 1 comment
Owner

Originally created by @craigfrancis on GitHub (Aug 22, 2021).

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']); // INSECURE

literal-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']);
Originally created by @craigfrancis on GitHub (Aug 22, 2021). ### Feature Request | Q | A |------------ | ------ | New Feature | yes | RFC | yes | BC Break | maybe #### Summary [Psalm 4.8](https://github.com/vimeo/psalm/releases/tag/4.8.0) 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. ```php $result = $qb ->select('u') ->from('User', 'u') ->where('u.id = ' . $_GET['id']); // INSECURE ``` `literal-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](https://github.com/doctrine/orm/blob/1de28c2cab5cd7e3c9d781a8beff22e328190911/lib/Doctrine/ORM/QueryBuilder.php#L696), the same could be used for `where()` [$predicates](https://github.com/doctrine/orm/blob/1de28c2cab5cd7e3c9d781a8beff22e328190911/lib/Doctrine/ORM/QueryBuilder.php#L1095). 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: ```php $result = $qb ->select('u') ->from('User', 'u') ->where('{my_field} = :my_value') ->setIdentifier('my_field', $_GET['field']) ->setParameter('my_value', $_GET['id']); ```
Author
Owner

@craigfrancis commented on GitHub (Sep 4, 2021):

The literal-string is also available in PHPStan 0.12.97

@craigfrancis commented on GitHub (Sep 4, 2021): The `literal-string` is also available in [PHPStan 0.12.97](https://github.com/phpstan/phpstan/releases/tag/0.12.97)
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6809