psalm icon indicating copy to clipboard operation
psalm copied to clipboard

Adding custom taint sources via plugin ignored

Open Patrick-Remy opened this issue 1 year ago • 10 comments

After upgrading from psalm 4.30.0 to psalm 5.13.1, I noticed that adding custom taint source via a psalm plugin is broken. Even the example at https://psalm.dev/docs/security_analysis/custom_taint_sources/ does not work.

Adding taint sources via docblock still works:

/**
  * @psalm-taint-source input
  */
function generateInput(): string {
    return 'dangerous';
}

/**
 * @psalm-taint-sink sql $sql
 */
function execSql($sql) {
}

execSql(generateInput());

This correctly throws ERROR: TaintedSql. But if I add taint sources via the plugin described at the page. No error is found for this snippet:

/**
 * @psalm-taint-sink sql $sql
 */
function execSql($sql) {
}

execSql($bad_data);

This is the exact code I used for the plugin. I have printed TAINTED to ensure, the variable really gets tainted.

<?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)
                );

                // Custom addition here to check if variable is really tainted
                echo "\n\n\nTAINTED\n\n\n";
            }
        }

        return null;
    }
}

Patrick-Remy avatar Jul 27 '23 11:07 Patrick-Remy