vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.54k stars 659 forks source link

Not taint specialization on object instantiation #8284

Open ohader opened 2 years ago

ohader commented 2 years ago

PoC: https://psalm.dev/r/49e01bb19a

// tainted HTML is correct here
$a = new Other($_GET['inject']);
echo $a->value;

// ... but it's a false-positive here
$b = new Other('static');
echo $b->value;
psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/49e01bb19a ```php value = $value; } } // tainted HTML is correct here $a = new Other($_GET['inject']); echo $a->value; // ... but it's a false-positive here $b = new Other('static'); echo $b->value; ``` ``` Psalm output (using commit 33b553e): ERROR: TaintedHtml - 14:6 - Detected tainted HTML ERROR: TaintedTextWithQuotes - 14:6 - Detected tainted text with possible quotes ERROR: TaintedHtml - 18:6 - Detected tainted HTML ERROR: TaintedTextWithQuotes - 18:6 - Detected tainted text with possible quotes ```
ohader commented 2 years ago

This is how it looks using @psalm-taint-specialize (but still, it's not feasible to adjust and manipulate each class in a particular project). https://psalm.dev/r/a3bf6ed8c6

psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/a3bf6ed8c6 ```php value = $value; } } // tainted HTML is correct here $a = new Other($_GET['inject']); echo $a->value; // ... but it's a false-positive here $b = new Other('static'); echo $b->value; ``` ``` Psalm output (using commit 33b553e): ERROR: TaintedHtml - 17:6 - Detected tainted HTML ERROR: TaintedTextWithQuotes - 17:6 - Detected tainted text with possible quotes ```
ohader commented 2 years ago

And this is how it would look like in framework-land, having builder, factories, facades triggering object instantiation. It seems all invocations have to use @psalm-taint-specialize.

https://psalm.dev/r/458f429c05

psalm-github-bot[bot] commented 2 years ago

I found these snippets:

https://psalm.dev/r/458f429c05 ```php value; $b = Factory::createOther('static'); echo $b->value; // ------------------------------------------------ class Factory { /** * @psalm-taint-specialize */ public static function createOther(string $value): Other { return new Other($value); } } /** * @psalm-taint-specialize */ class Other { public readonly string $value; public function __construct(string $value) { $this->value = $value; } } ``` ``` Psalm output (using commit 33b553e): ERROR: TaintedHtml - 8:6 - Detected tainted HTML ERROR: TaintedTextWithQuotes - 8:6 - Detected tainted text with possible quotes ```