Track taints through Twig attributes, loops, assignments and includes - #390
Merged
Merged
Conversation
This was referenced Oct 2, 2026
Open
danog
force-pushed
the
twig-taint-expressions
branch
from
October 5, 2026 08:44
115609b to
f19facc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #389 (Psalm 7 support), whose two commits come first here.
The Twig taint analysis only saw a variable printed as is (
{{ untrusted|raw }}), so most of the ways a template displays its parameters went unnoticed. Each of the following was missed, and is now reported:{{ user.name|raw }},{{ ('Hello ' ~ name)|raw }},{{ format(value)|raw }}: a printed expression takes the taints of the variables it is made of, through its filters, and anescape/efilter anywhere in it escapes what it applies to.{{ (untrusted ? 'a' : 'b')|raw }}and{{ (untrusted == 'x')|raw }}were reported, though only a constant or a boolean is displayed: the condition of a ternary, and the expressions giving a boolean or a number (comparisons, tests,not, arithmetic), don't carry the taints of what they are made of. A branch still does.{% for item in items %}{{ item|raw }}{% endfor %}: the loop variables take the taints of what is looped over.{% set greeting = 'Hello ' ~ name %}:setwas only followed when assigning another variable.{{ untrusted|raw }}followed by{{ untrusted }}: each use of a template variable replaced the previous one as the node its taints flow into, so only the last use was connected. Every use now goes through the node of the first one.{% include 'part.html.twig' %}: an included template is given the including template's variables (those it reads from its context, unless the include isonly) and those ofwith, and what it outputs is part of what the including template outputs. Only includes with a constant template name are followed.TemplateFileAnalyzer, and reported parse errors for those containing<?php, such as templates generating PHP code.TemplateFileScanner, configured as the scanner of the.twigextension next to the checker (see the README), skips that.twigRootPathcrashed the analysis (the loader only knew the root); it is now named after its path in the project, which is also how other frameworks' renderers can refer to it.Tests: 13 new scenarios in
TwigTaintingWithAnalyzer.feature; the 7 expecting errors, and the one with a template containing<?php, fail without this change.phpunit(45 tests),codecept run -g symfony-common(106 tests) andpsalm --find-dead-code --find-unused-psalm-suppresspass.