nette / latte

☕ Latte: the safest & truly intuitive templates for PHP. Engine for those who want the most secure PHP sites.
https://latte.nette.org
Other
1.09k stars 107 forks source link

[Feature Request] Check StringLoader receives `literal-string` values. #331

Closed craigfrancis closed 1 year ago

craigfrancis commented 1 year ago

While I would hope this is rare, I have seen a junior developer edit a string template to include untrusted user data, something like the following:

$latte->setLoader(new Latte\Loaders\StringLoader([
        'example' => '<img src="{$src}" class="' . $_GET['type'] . '">',
    ]));

Both Psalm 4.8 and PHPStan 0.12.97 have provided the literal-string type (since September 2021), and it's recognised by PhpStorm 2022.3.

This type allows you to note certain methods expect a string that has been written by the developer (i.e. a string defined in the source code; where it does not include any untrusted user data).

It's also permitted for multiple literal-string values to be concatenated together.

Would it be possible to update the StringLoader to use something like:

    /**
-    * @param  string[]  $templates
+    * @param  array<string, literal-string>|null  $templates
     */
    public function __construct(?array $templates = null)

It might also be used elsewhere, e.g. the $name in Engine::render(), but I appreciate that method is used in multiple ways (maybe a database record specifies which template to render), so this might not be appropreate:

$latte->render('<img src="{$src}" class="' . $_GET['type'] . '">', ['src' => 'https://...']);
    /**
     * Renders template to output.
+    * @param  literal-string  $name
     * @param  object|mixed[]  $params
     */
    public function render(string $name, object|array $params = [], ?string $block = null): void
dg commented 1 year ago

I use the literal-string type with nette/database, where the vast majority of queries are really literals (and SQL injection can be caused by a simple overlook), but StringLoader can be used in many different ways and forcing literal there seems very restrictive to me.

mabar commented 1 year ago

You can always add it yourself via phpstan stubs, to enforce project-specific rules https://phpstan.org/user-guide/stub-files

craigfrancis commented 1 year ago

You're probably right, I only get to see a sub-set of developers using it, and the few times they use StringLoader it's been with an array of literal-string values (that said, I would be intrigued to see those examples).

And good point Marek, I'll suggest that to the developers, to run those checks locally.