Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Taint analysis: allow marking properties as taint sources? #10896

Open
Ocramius opened this issue Apr 11, 2024 · 2 comments
Open

Taint analysis: allow marking properties as taint sources? #10896

Ocramius opened this issue Apr 11, 2024 · 2 comments

Comments

@Ocramius
Copy link
Contributor

I've snooped around taint analysis, which I found to be useful in very legacy projects that heavily on superglobals like $_GET, $_SESSION, etc.

In more recent / modern projects:

  • most DB interactions are hidden behind a DAO / ORM that operates with a Id<T> -> object<T> API (think Doctrine ORM), where record/entity objects hold user data
  • most fields that hold input data are on objects with public readonly fields

The current taint analysis only operates with taint sources being function-alike nodes:

I'm wondering if it makes sense to allow object properties to be marked as taint sources.

Copy link

Hey @Ocramius, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

@Ocramius
Copy link
Contributor Author

Seems like this is achievable via custom taint sources:

## Custom taint plugin
For example this plugin treats all variables named `$bad_data` as taint sources.
```php
<?php
namespace Some\Ns;
use PhpParser;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\FileManipulation;
use Psalm\Plugin\EventHandler\AfterExpressionAnalysisInterface;
use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent;
use Psalm\Type\TaintKindGroup;
class BadSqlTainter implements AfterExpressionAnalysisInterface
{
/**
* Called after an expression has been checked
*
* @param PhpParser\Node\Expr $expr
* @param Context $context
* @param FileManipulation[] $file_replacements
*
* @return void
*/
public static function afterExpressionAnalysis(AfterExpressionAnalysisEvent $event): ?bool {
$expr = $event->getExpr();
$statements_source = $event->getStatementsSource();
$codebase = $event->getCodebase();
if ($expr instanceof PhpParser\Node\Expr\Variable
&& $expr->name === 'bad_data'
) {
$expr_type = $statements_source->getNodeTypeProvider()->getType($expr);
// should be a globally unique id
// you can use its line number/start offset
$expr_identifier = '$bad_data'
. '-' . $statements_source->getFileName()
. ':' . $expr->getAttribute('startFilePos');
if ($expr_type) {
$codebase->addTaintSource(
$expr_type,
$expr_identifier,
TaintKindGroup::ALL_INPUT,
new CodeLocation($statements_source, $expr)
);
}
}
return null;
}
}
```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant