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.

Sign up for GitHub

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

Jump to bottom

Adding custom taint sources via plugin ignored #10057

Open
Patrick-Remy opened this issue Jul 27, 2023 · 10 comments · May be fixed by #10206 or #11025
Open

Adding custom taint sources via plugin ignored #10057

Patrick-Remy opened this issue Jul 27, 2023 · 10 comments · May be fixed by #10206 or #11025
Labels
taint analysis

Comments

@Patrick-Remy
Copy link
Contributor

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
Copy link
Contributor Author

Patrick-Remy commented Jul 27, 2023 •
edited
Loading

Also using the AddTaintsInterface doesn't make a difference:

<?php

namespace Some\Ns;

use PhpParser;
use Psalm\Plugin\EventHandler\AddTaintsInterface;
use Psalm\Plugin\EventHandler\Event\AddRemoveTaintsEvent;
use Psalm\Type\TaintKindGroup;

class BadSqlTainter implements AddTaintsInterface
{

    /**
     * Called to see what taints should be added
     *
     * @return list<string>
     */
    public static function addTaints(AddRemoveTaintsEvent $event): array
    {
        $expr = $event->getExpr();
        if ($expr instanceof PhpParser\Node\Expr\Variable
            && $expr->name === 'bad_data'
        ) {
            echo "\n\n\nTAINTED\n\n\n";
            return TaintKindGroup::ALL_INPUT;
        }

        return [];
    }
}

@Patrick-Remy
Copy link
Contributor Author

Patrick-Remy commented Jul 27, 2023 •
edited
Loading

Just checked again, with the above plugin using AfterExpressionAnalysisInterface, on v4.30.0 errors get detected, using v5.0.0 no errors are found.

Interestingly, even on v4.30.0 no errors are found using the AddTaintsInterface.

If I debug the taint graph with the different versions. The TaintGraph in v5.0.0+ has much less edges for the few lines of the test snippet.

@Patrick-Remy
Copy link
Contributor Author

Still reproducible with current dev-master. I created a repository for easy reproduction: https://github.com/Patrick-Remy/repro-psalm-5-taint-analysis-issue/blob/master/README.md

@weirdan do you have an idea why the TaintGraph has so big changed between v4 and v5?

@weirdan
Copy link
Collaborator

weirdan commented Sep 7, 2023

do you have an idea why the TaintGraph has so big changed between v4 and v5?

I don't think there were that many changes between v4 and v5, apart from project-wide stylistic changes (we applied several automatic refactorings when we bumped our minimum PHP version).

@Patrick-Remy
Copy link
Contributor Author

Patrick-Remy commented Sep 7, 2023 •
edited
Loading

In July I had a bigger source file when diffing the TaintFlowGraphs.
I tried to debug it again and dumped in the minimal repro afterconnectSinksAndSources in Analyzer. And the main diff seems a missing edge in forward_edges with length 0

Psalm v4 
object(Psalm\Internal\Codebase\TaintFlowGraph)#27 (6) {
  ["forward_edges":protected]=>
  array(2) {
    ["$bad_data-example.php:81"]=>
    array(1) {
      ["call to execSql-example.php:81-89"]=>
      object(Psalm\Internal\DataFlow\Path)#85611 (4) {
        ["type"]=>
        string(3) "arg"
        ["unescaped_taints"]=>
        array(0) {
        }
        ["escaped_taints"]=>
        array(0) {
        }
        ["length"]=>
        int(0)
      }
    }
    ["call to execSql-example.php:81-89"]=>
    array(1) {
      ["execsql#1"]=>
      object(Psalm\Internal\DataFlow\Path)#85610 (4) {
        ["type"]=>
        string(3) "arg"
        ["unescaped_taints"]=>
        array(0) {
        }
        ["escaped_taints"]=>
        array(0) {
        }
        ["length"]=>
        int(3)
      }
    }
  }
  ["sources":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(1) {
    ["$bad_data-example.php:81"]=>
    object(Psalm\Internal\DataFlow\TaintSource)#85607 (9) {
      ["id"]=>
      string(24) "$bad_data-example.php:81"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(24) "$bad_data-example.php:81"
      ["code_location"]=>
      object(Psalm\CodeLocation)#85606 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(9)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(9)
        ["raw_file_start"]=>
        int(81)
        ["raw_file_end"]=>
        int(89)
        ["file_start":protected]=>
        int(81)
        ["file_end":protected]=>
        int(89)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(73)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(92)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(81)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(90)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(9)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(18)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(19) "execSql($bad_data);"
        ["text":"Psalm\CodeLocation":private]=>
        string(9) "$bad_data"
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(true)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(13) {
        [0]=>
        string(4) "html"
        [1]=>
        string(10) "has_quotes"
        [2]=>
        string(5) "shell"
        [3]=>
        string(3) "sql"
        [4]=>
        string(8) "callable"
        [5]=>
        string(4) "eval"
        [6]=>
        string(11) "unserialize"
        [7]=>
        string(7) "include"
        [8]=>
        string(4) "ssrf"
        [9]=>
        string(4) "ldap"
        [10]=>
        string(4) "file"
        [11]=>
        string(6) "header"
        [12]=>
        string(6) "cookie"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["nodes":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(3) {
    ["execsql#1"]=>
    object(Psalm\Internal\DataFlow\TaintSink)#85608 (9) {
      ["id"]=>
      string(9) "execsql#1"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(9) "execSql#1"
      ["code_location"]=>
      object(Psalm\CodeLocation)#13350 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(6)
        ["raw_file_start"]=>
        int(62)
        ["raw_file_end"]=>
        int(65)
        ["file_start":protected]=>
        int(62)
        ["file_end":protected]=>
        int(65)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(45)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(69)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(62)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(66)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(18)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(22)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(24) "function execSql($sql) {"
        ["text":"Psalm\CodeLocation":private]=>
        string(4) "$sql"
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        int(5)
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(true)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(1) {
        [0]=>
        string(3) "sql"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
    ["call to execSql-example.php:81-89"]=>
    object(Psalm\Internal\DataFlow\DataFlowNode)#85609 (9) {
      ["id"]=>
      string(33) "call to execSql-example.php:81-89"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(15) "call to execSql"
      ["code_location"]=>
      object(Psalm\CodeLocation)#85600 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(9)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(9)
        ["raw_file_start"]=>
        int(81)
        ["raw_file_end"]=>
        int(89)
        ["file_start":protected]=>
        int(81)
        ["file_end":protected]=>
        int(89)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(73)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(92)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(81)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(90)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(9)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(18)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(19) "execSql($bad_data);"
        ["text":"Psalm\CodeLocation":private]=>
        string(9) "$bad_data"
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(true)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(0) {
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
    ["execsql"]=>
    object(Psalm\Internal\DataFlow\DataFlowNode)#85613 (9) {
      ["id"]=>
      string(7) "execsql"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(7) "execsql"
      ["code_location"]=>
      object(Psalm\CodeLocation)#13448 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(54)
        ["raw_file_end"]=>
        int(60)
        ["file_start":protected]=>
        int(54)
        ["file_end":protected]=>
        int(60)
        ["single_line":protected]=>
        bool(true)
        ["preview_start":protected]=>
        int(54)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(0) {
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["sinks":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(1) {
    ["execsql#1"]=>
    object(Psalm\Internal\DataFlow\TaintSink)#85608 (9) {
      ["id"]=>
      string(9) "execsql#1"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(9) "execSql#1"
      ["code_location"]=>
      object(Psalm\CodeLocation)#13350 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(6)
        ["raw_file_start"]=>
        int(62)
        ["raw_file_end"]=>
        int(65)
        ["file_start":protected]=>
        int(62)
        ["file_end":protected]=>
        int(65)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(45)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(69)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(62)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(66)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(18)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(22)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(24) "function execSql($sql) {"
        ["text":"Psalm\CodeLocation":private]=>
        string(4) "$sql"
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        int(5)
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(true)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(1) {
        [0]=>
        string(3) "sql"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["specialized_calls":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(0) {
  }
  ["specializations":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(0) {
  }
}
Psalm v5 
object(Psalm\Internal\Codebase\TaintFlowGraph)#30 (6) {
  ["forward_edges":protected]=>
  array(1) {
    ["call to execSql-example.php:81-89"]=>
    array(1) {
      ["execsql#1"]=>
      object(Psalm\Internal\DataFlow\Path)#97446 (4) {
        ["type"]=>
        string(3) "arg"
        ["unescaped_taints"]=>
        array(0) {
        }
        ["escaped_taints"]=>
        array(0) {
        }
        ["length"]=>
        int(3)
      }
    }
  }
  ["sources":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(1) {
    ["$bad_data-example.php:81"]=>
    object(Psalm\Internal\DataFlow\TaintSource)#97439 (9) {
      ["id"]=>
      string(24) "$bad_data-example.php:81"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(24) "$bad_data-example.php:81"
      ["code_location"]=>
      object(Psalm\CodeLocation)#97435 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(9)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(81)
        ["raw_file_end"]=>
        int(89)
        ["file_start":protected]=>
        int(81)
        ["file_end":protected]=>
        int(89)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(81)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":protected]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(13) {
        [0]=>
        string(4) "html"
        [1]=>
        string(10) "has_quotes"
        [2]=>
        string(5) "shell"
        [3]=>
        string(3) "sql"
        [4]=>
        string(8) "callable"
        [5]=>
        string(4) "eval"
        [6]=>
        string(11) "unserialize"
        [7]=>
        string(7) "include"
        [8]=>
        string(4) "ssrf"
        [9]=>
        string(4) "ldap"
        [10]=>
        string(4) "file"
        [11]=>
        string(6) "header"
        [12]=>
        string(6) "cookie"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["nodes":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(3) {
    ["execsql#1"]=>
    object(Psalm\Internal\DataFlow\TaintSink)#97443 (9) {
      ["id"]=>
      string(9) "execsql#1"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(9) "execSql#1"
      ["code_location"]=>
      object(Psalm\CodeLocation)#16172 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(62)
        ["raw_file_end"]=>
        int(65)
        ["file_start":protected]=>
        int(62)
        ["file_end":protected]=>
        int(65)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(62)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":protected]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        int(5)
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(1) {
        [0]=>
        string(3) "sql"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
    ["call to execSql-example.php:81-89"]=>
    object(Psalm\Internal\DataFlow\DataFlowNode)#97445 (9) {
      ["id"]=>
      string(33) "call to execSql-example.php:81-89"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(15) "call to execSql"
      ["code_location"]=>
      object(Psalm\CodeLocation)#97438 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(9)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(81)
        ["raw_file_end"]=>
        int(89)
        ["file_start":protected]=>
        int(81)
        ["file_end":protected]=>
        int(89)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(81)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":protected]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(0) {
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
    ["execsql"]=>
    object(Psalm\Internal\DataFlow\DataFlowNode)#97448 (9) {
      ["id"]=>
      string(7) "execsql"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(7) "execsql"
      ["code_location"]=>
      object(Psalm\CodeLocation)#16364 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(54)
        ["raw_file_end"]=>
        int(60)
        ["file_start":protected]=>
        int(54)
        ["file_end":protected]=>
        int(60)
        ["single_line":protected]=>
        bool(true)
        ["preview_start":protected]=>
        int(54)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":protected]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        NULL
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(0) {
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["sinks":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(1) {
    ["execsql#1"]=>
    object(Psalm\Internal\DataFlow\TaintSink)#97443 (9) {
      ["id"]=>
      string(9) "execsql#1"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(9) "execSql#1"
      ["code_location"]=>
      object(Psalm\CodeLocation)#16172 (23) {
        ["file_path"]=>
        string(77) "....../example.php"
        ["file_name"]=>
        string(11) "example.php"
        ["raw_line_number"]=>
        int(6)
        ["end_line_number":"Psalm\CodeLocation":private]=>
        int(-1)
        ["raw_file_start"]=>
        int(62)
        ["raw_file_end"]=>
        int(65)
        ["file_start":protected]=>
        int(62)
        ["file_end":protected]=>
        int(65)
        ["single_line":protected]=>
        bool(false)
        ["preview_start":protected]=>
        int(62)
        ["preview_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_start":"Psalm\CodeLocation":private]=>
        int(-1)
        ["selection_end":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_from":"Psalm\CodeLocation":private]=>
        int(-1)
        ["column_to":"Psalm\CodeLocation":private]=>
        int(-1)
        ["snippet":"Psalm\CodeLocation":private]=>
        string(0) ""
        ["text":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_start"]=>
        NULL
        ["docblock_start_line_number":"Psalm\CodeLocation":private]=>
        NULL
        ["docblock_line_number":protected]=>
        NULL
        ["regex_type":"Psalm\CodeLocation":private]=>
        int(5)
        ["have_recalculated":"Psalm\CodeLocation":private]=>
        bool(false)
        ["previous_location"]=>
        NULL
      }
      ["specialization_key"]=>
      NULL
      ["taints"]=>
      array(1) {
        [0]=>
        string(3) "sql"
      }
      ["previous"]=>
      NULL
      ["path_types"]=>
      array(0) {
      }
      ["specialized_calls"]=>
      array(0) {
      }
    }
  }
  ["specialized_calls":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(0) {
  }
  ["specializations":"Psalm\Internal\Codebase\TaintFlowGraph":private]=>
  array(0) {
  }
}

@Patrick-Remy
Copy link
Contributor Author

Patrick-Remy commented Sep 8, 2023 •
edited
Loading

The issue seems to be caused by d0be59e#diff-122c0df94b9a0557e4fd09b8d8c7324f4d6d8fff34f344e1e49318c8e7a0a242

With the commit before (51838a545) the taint is found, when checking out d0be59e16, the issues are ignored.
The ArgumentAnalyzer::verifyType gets called with $arg_value_type as $input_type, When dumping $input_type->parent_nodes, it is empty with d0be59e16 but isn't with the commit before.

Dump with 51838a545

 ["parent_nodes"]=>
  array(1) {
    ["$bad_data-example.php:81"]=>
    object(Psalm\Internal\DataFlow\TaintSource)#90618 (9) {
      ["id"]=>
      string(24) "$bad_data-example.php:81"
      ["unspecialized_id"]=>
      NULL
      ["label"]=>
      string(24) "$bad_data-example.php:81"

So anywhere the parent nodes get lost

@Patrick-Remy
Copy link
Contributor Author

Patrick-Remy commented Sep 8, 2023 •
edited
Loading

I think I got it, with the change of immutability, addTaintSource() doesn't manipulate the $expr_type anymore and instead returns the adjusted $expr_type. But this won't work, as from a plugin perspective I cannot adjust the expression type anymore inside the plugin.

So probably that's expected by you, but then there is the question why the addTaints() also doesn't work.

@Patrick-Remy Patrick-Remy linked a pull request Sep 15, 2023 that will close this issue
Open
@Patrick-Remy
Copy link
Contributor Author

See my PR for furhter analysis/discussion

@cgocast
Copy link
Contributor

cgocast commented Jun 5, 2024 •
edited
Loading

I ran into the same bug while trying to add $_FILES and $argv as taint sources. As a workaround, I patched psalm with
this file

@cgocast cgocast linked a pull request Jun 24, 2024 that will close this issue
Open
@cgocast
Copy link
Contributor

cgocast commented Jun 25, 2024

In the PR mentionned above, I take another approach than @Patrick-Remy in order to solve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
taint analysis
Projects
None yet
Milestone
No milestone
3 participants
@weirdan @Patrick-Remy @cgocast