Add support for literal-string type, to address Injection Vulnerabilities

[craigfrancis]

Feature request

Psalm 4.8 has introduced a new literal-string type, which is used to "distinguish strings from a trusted developer, from strings that may be attacker controlled".

This helps identify the main source of Injection Vulnerabilities, where developers incorrectly include user-input in sensitive strings (SQL, HTML, CLI, etc).

/** @param literal-string $sql */
function execute_sql(string $sql, array $params = []): void { }

$id = (string) ($_GET['id'] ?? '');

// passes type checks
execute_sql(
    'SELECT * FROM `foo` WHERE `id` = :id',
    [':id' => $id]
);

// fails
execute_sql(
    'SELECT * FROM `foo` WHERE `id` = "' .$id . '"'
);

It is based on the is_literal() RFC that didn't make it in for PHP 8.1, but will be re-considered for 8.2.

---

As an aside, this is not the same as Taint Checking, which incorrectly assumes the output of an escaping function is “safe” for a particular context (creating a false sense of security); for example:

$sql = 'SELECT * FROM users WHERE id = ' . $db->real_escape_string($id); // INSECURE, $id = "id"
$html = "<img src=" . htmlentities($url) . " alt='' />"; // INSECURE, $url = "/ onerror=alert(1)"
$html = "<a href='" . htmlentities($url) . "'>..."; // INSECURE, $url = "javascript:alert(1)"; and single quotes before PHP 8.1 were not escaped by default.

Did PHPStan help you today? Did it make you happy in any way?

PHPStan helps me focus on problem solving, knowing that it will pick up most typos/mistakes (the reason for this feature request is to catch the last set of mistakes that I still worry about, especially when working with a larger team of developers).

[ondrejmirtes]

Hi, please send a PR that interprets literal-string as new StringType for maximum compatibility: https://github.com/phpstan/phpstan-src/blob/4e7d6c155c52e11f81bcabf75f84e14a2e0e1727/src/PhpDoc/TypeNodeResolver.php#L192-L202

To fully support this and introduce some checks would be much more work, I have no plans to do that currently.

[craigfrancis]

Thanks for the pointers Ondřej. I’ll make a PR (it might take me a few weeks, as I’m not familiar with the internals of PHPStan, and I’ve got some other work to be doing as well).

[ondrejmirtes]

No problem, it's literally two lines of code :)

[craigfrancis]

Sorry, I didn't read your message correctly... I've created a PR to add 'literal-string' as a basic StringType().

To fully support...

Would you say it's more of an IntersectionType like 'numeric-string' (StringType + AccessoryNumericStringType); or should it extend StringType, like how 'class-string' (ClassStringType)?

I suspect extending would be easier, so it can contain a flag to say if it was defined in the developers source code... I wonder if ConstantStringType is the key here, which has the isClassString private property.

[ondrejmirtes]

This is implemented :)

[dktapps]

FWIW, this is approximately the same as #4151 (for the sake of posterity).

[craigfrancis]

Thanks @dktapps, I'm hoping to bring this concept (identifying programmer defined strings to stop Injection Vulnerabilities) to a few different projects, and generally raise awareness - if you want to discuss it further, or help out, feel free to email me off-list.

[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.