Conditional sink support

[LukasReschke]

e.g. print_r is only a sink depending on the second parameter.

Probably possible to take the logic from https://github.com/vimeo/psalm/commit/af008953a8022566477b0555a10e17dac58e6a2f and apply it somewhat adjusted to psalm-taint-sink.

Ref https://github.com/vimeo/psalm/issues/3665

[muglug]

It'll be

/**
 * @param mixed $var
 * @param bool $return
 * @return ($return is true ? string : true)
 *
 * @psalm-taint-specialize
 * @psalm-flow ($var) -> return
 * @psalm-taint-sink ($return is false ? 'html' : null) $var
 */
function print_r($var, bool $return = false) {}

Same for var_dump

[ohader]

I just realized that my previous approach in #5095 does not make much sense - instead I should have been used conditional tainted sinks like described in this issue.

The adjusted use-case of #5095 then probably would look like this: https://psalm.dev/r/274b78511c

I tried to kick-start this feature, but struggled when actually inferencing and assigning values in FunctionLikeDocblockScanner and FunctionLikeParameter - I could need some help here.

The current (unfinished) change is available at: https://github.com/vimeo/psalm/compare/master...ohader:4669_conditional_taint_sinks?expand=1

I'm currently working on a Psalm integration the TYPO3 project - in case somebody would like to pick of this task and sponsoring (payment) helps to get it done and merged to vimeo/psalm, please get in contact with me directly ([email protected] or https://keybase.io/oliverhader). Thx

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/274b78511c

<?php // --taint-analysis

/**
 * @param string $value used as command argument
 * @param bool $escape whether to escape $value
 *
 * @psalm-taint-sink ($escape is false ? 'shell' : null) $value
 */
function doThat(string $value, bool $escape = true): void {}

doThat((string)($_GET['inject'] ?? ''), true); // safe
doThat((string)($_GET['inject'] ?? ''), false); // unsafe

Psalm output (using commit 3ce41d7):

No issues!

[emmanuelGuiton]

I think we should not consider that using print_r function with $result=true is always safe. It depends on what we do with the result. echo print_r($_GET, true); could be as harmful as print_r($_GET);

[ohader]

@emmanuelGuiton correct, it's not always safe... the current stub is strict here, the 1st parameter is always considered harmful - which leads to false-positives for $result = print_r($_GET, true) and $result never being used at all later on...

I've got a custom implementation which looks like

    /**
     * @param mixed $var
     * @param bool $return
     * @return ($return is true ? string : true)
     *
     * @psalm-taint-specialize
     * @typo3-taint-sink ($return is false ? 'html' : null) $var
     * @typo3-taint-source ($return is true ? 'input' : null)
     */
    function print_r($var, bool $return = false) {}

• conditional sink handling, depending on $return
• conditional source handling (return value of print_r considered as source), depending on $return (side-effect: overrides semantics of original $var source)
• (not implemented) e.g. conditional @psalm-flow ($var) -> ($return is true ? 'return' : null) might be useful here

(I tried to integrate that handling into Psalms core sources, but failed - thus, it's part of a custom plugin currently)

[abarker-gravity]

Alright. For those of you that end up on this bug report looking for a workaround and are frustrated with the lack of documentation, please follow these steps:

1. Create a new plugin class. Please note that this file will not resolve using the <psalm autoloader="" you may have specified in your psalm.xml configuration file. IT MUST BE LOADED BY COMPOSER'S AUTOLOADER!

<?php
   
namespace Your\Project\Psalm;

use Psalm\Plugin\PluginEntryPointInterface;
use Psalm\Plugin\RegistrationInterface;
use SimpleXMLElement;

class LocalPlugin implements PluginEntryPointInterface
{
    public function __invoke(RegistrationInterface $psalm, ?SimpleXMLElement $config = null): void
    {
        $psalm->addStubFile(__DIR__ . '/CoreGenericBugfix.phpstub');
    }
}

2. Create a new CoreGenericBugfix.phpstub definition at the same location you placed the class from step 1.

/**
 * @param mixed $var
 * @param bool $return
 * @return ($return is true ? string : true)
 *
 * @psalm-taint-specialize
 * @typo3-taint-sink ($return is false ? 'html' : null) $var
 * @typo3-taint-source ($return is true ? 'input' : null)
 */
function print_r($var, bool $return = false) {}

3. Update your psalm.xml file to load this new plugin. Please note that you must use <pluginClass class="" as <plugin filename="" does not work with either a stub file or class.

<?xml version="1.0"?>
<psalm autoloader="bootstrap.php">
    ...
    <plugins>
        <pluginClass class="Your\Project\Psalm\LocalPlugin" />
    </plugins>
</psalm>