taint on empty strings

[staabm]

I dont expect a error in this case

https://psalm.dev/r/a020e4ddc2

as soon as I drop the if ($file) { it works like expected.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/a020e4ddc2

<?php // --taint-analysis

    function importScript(string $filename)
    {
        if (is_file($filename)) {
            $c = file_get_contents($filename);
        }
    }


$file = $_GET['filename'];

if ($file != '') {
    
    /**
     * @psalm-taint-escape text
     */
    $file = basename($file);
}

importScript($file);

Psalm output (using commit 95de6cf):

ERROR: TaintedText - 6:36 - Detected tainted text

[staabm]

the initially reported taint error is still reproducible

https://psalm.dev/r/a020e4ddc2

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/a020e4ddc2

<?php // --taint-analysis

    function importScript(string $filename)
    {
        if (is_file($filename)) {
            $c = file_get_contents($filename);
        }
    }


$file = $_GET['filename'];

if ($file != '') {
    
    /**
     * @psalm-taint-escape text
     */
    $file = basename($file);
}

importScript($file);

Psalm output (using commit ccf6e28):

ERROR: TaintedFile - 6:36 - Detected tainted file handling

[weirdan]

the initially reported taint error is still reproducible

At the very least it's titled misleadingly - there's no guarantee that $_GET['filename'] contains a string, as evidenced by type errors reported by Psalm: https://psalm.dev/r/d25318bfb9

If you make sure it's a string (e.g. by using strict comparison) the taint is gone: https://psalm.dev/r/3fd7e9379c

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/d25318bfb9

<?php // --no-taint-analysis

    function importScript(string $filename): void
    {
        if (is_file($filename)) {
            $c = file_get_contents($filename);
        }
    }

if (!isset($_GET['filename'])) throw new RuntimeException('missing');
$file = $_GET['filename'];

if ($file != '') {
    
    /**
     * @psalm-taint-escape text
     */
    $file = basename($file);
}

importScript($file);

Psalm output (using commit ccf6e28):

INFO: MixedAssignment - 11:1 - Unable to determine the type that $file is being assigned to

INFO: MixedArgument - 18:22 - Argument 1 of basename cannot be mixed, expecting string

INFO: MixedArgument - 21:14 - Argument 1 of importScript cannot be mixed|string, expecting string

https://psalm.dev/r/3fd7e9379c

<?php // --taint-analysis

    function importScript(string $filename): void
    {
        if (is_file($filename)) {
            $c = file_get_contents($filename);
        }
    }

if (!isset($_GET['filename'])) throw new RuntimeException('missing');
$file = $_GET['filename'];

if ($file !== '') {
    
    /**
     * @psalm-taint-escape text
     */
    $file = basename($file);
}

importScript($file);

Psalm output (using commit ccf6e28):

No issues!