Custom taint sinks added programmaticaly

[adrienlucas]

Hi !

I'm working with the psalm symfony plugin, and I want to be able to add a taint sink programmaticaly. When inside a hook, you can add a taint source by calling Codebase::addTaintSource, but on the other hand, adding a taint sink is not that easy.

I tried to implement my own method, but it does not work and maybe i'm totally going the wrong way.. Is there actually an other technique to achieve this ? Or is it just a mistake in my implementation ?

Here is the implementation I tried (it's basically a copy-paste of addTaintSource) :

private function addTaintSink(Codebase $codebase, Type\Union $expr_type, string $taint_id, array $taints = \Psalm\Type\TaintKindGroup::ALL_INPUT, ?CodeLocation $code_location = null) : void
{
        if (!$codebase->taint) {
            return;
        }

        $sink = new \Psalm\Internal\Taint\Sink($taint_id, $taint_id, $code_location, null, $taints);

        $codebase->taint->addSink($sink);

        $expr_type->parent_nodes = [
            $sink,
        ];
}

[muglug]

I tried to implement my own method, but it does not work

I've just added one here: https://github.com/vimeo/psalm/blob/d950ddfff66fa199f1f7f3dce85cdb108d199940/src/Psalm/Codebase.php#L1763-L1785

One big thing is missing – a path to think sink. For input to end up there, there needs to be some way that tainted input can flow to the sink.

Can you describe the sink you're trying to add so I can better understand the problem?

[adrienlucas]

Can you describe the sink you're trying to add so I can better understand the problem?

Within the Symfony framework, one can render a template with parameters using AbstractController::render(string $templateName, array $parameters): string. If one of the parameters is displayed in the template with auto-escaping disabled (this logic is related to the templating engine, I still have to write it), the method should be considered a sink.

[adrienlucas]

One big thing is missing – a path to think sink. For input to end up there, there needs to be some way that tainted input can flow to the sink.

I'm not fully understanding the way it all work, but are you saying that after the sink is added to the $codebase, if should trigger some thing to have the source and sink connected ?

Edit : ok i think i get what you meant, and actually i didn't think this through.. So, the first step (adding the sink) should be ok now with the method you added, now i have to put some logic to actually check that the source flows to the sink, right ?

[mortenson]

I'm having trouble using this - to debug, I tried adding a taint to every expression, but could never trigger an error. I would expect code similar to this to throw a lot of false positives:

class Hooks implements AfterExpressionAnalysisInterface
{

    public static function afterExpressionAnalysis(
        Expr $expr,
        Context $context,
        StatementsSource $statements_source,
        Codebase $codebase,
        array &$file_replacements = []
    ): ?bool {
        $expr_id = 'test'
        . '-' . $statements_source->getFileName()
        . ':' . $expr->getAttribute('startFilePos');
        $codeLocation = new CodeLocation($statements_source, $expr);
        $codebase->addTaintSink($expr_id, TaintKindGroup::ALL_INPUT, $codeLocation);
        return null;
    }

}

Any thoughts?

[weirdan]

@mortenson I'm not very familiar with Psalm's taint analysis, but could it perhaps be the case that you're adding the sink too late? I would expect sinks to be added before analysis, not after.

[mortenson]

@weirdan I was basing this on the hook used by the taint source docs https://psalm.dev/docs/security_analysis/custom_taint_sources, since there's not a reference example for addTaintSink yet.

[ohader]

This issue also sound like a documentation task, describing the taint graph, how to add taints and sinks and finally how to understand edges & co.

[orklah]

tag added :)

[ohader]

Issue created #7544 😄