escape modifier on variables with nofilter has no effect when auto-escaping is enabled

[turtlexiv]

PR #1030 introduced changes to the escape modifier to prevent double-escaping when auto-escaping is enabled.

However, this change also applies to template variables where the nofilter flag is used. This results in the counter-intuitive behavior that explicitly calling the escape modifier when using nofilter has no effect. In our use cases, we use nofilter combined with an explicit escape when we're applying HTML-producing modifiers to unescaped data; for example, {$description|escape|nl2br nofilter}. The net result of this change is 5.4.0 introducing immediate security issues in our application.

This behavior is not reflected in the documentation at https://www.smarty.net/docs/en/language.modifier.escape.tpl or https://smarty-php.github.io/smarty/stable/designers/language-modifiers/language-modifier-escape/, nor is the new force option.

When using nofilter, explicit uses of escape probably should apply.

Thoughts @Amaury ?

[Amaury]

Hi @turtlexiv. Thanks for the report and the clean reproduction. You're right, this is a genuine security regression in 5.4.0, not intended behavior. I'll own this one.

Root cause

My PR turned the escape modifier (default html mode) into a no-op when auto-escaping is globally enabled, on the assumption that the global auto-escaping would escape the value anyway, which avoids double-escaping. That assumption breaks whenever the global auto-escaping does not actually run on the value:

• with nofilter or the |raw modifier, the outer auto-escaping is skipped, and since escape was turned into a no-op, nothing escapes the value at all. That is the XSS you hit with {$description|escape|nl2br nofilter}.
• with a following filter, e.g. {$var|escape|nl2br}, the only escaping left runs after nl2br, so it also escapes the <br /> that nl2br produced (functional breakage rather than XSS).

Why I missed it

In Smarty 4, nofilter was documented on the $escape_html page as the way to disable auto-escaping. In Smarty 5 that page was removed from the mkdocs.yml navigation, so it is no longer part of the published documentation, even though the compiler still honors nofilter. The only place nofilter still appears in the published Smarty 5 docs is the {setfilter} page, which is a completely different use case. So when I wrote the PR, nofilter looked like it was no longer a documented feature of auto-escaping, and I did not account for it. The docs should be restored on that point.

An approach that does not work, and why

The obvious fix is to make the no-op aware of nofilter: skip the explicit escape only when auto-escaping is on AND nofilter is not set. I implemented and tested it, and it cannot work with the current compiler, for an ordering reason.

nofilter is a tag-level option flag, processed in PrintExpressionCompiler. But the expression's modifiers (escape, nl2br) are compiled earlier, during expression parsing (compileModifierInExpression), before PrintExpressionCompiler runs and before nofilter is read. By the time the compiler knows nofilter, escape has already been compiled (and already no-op'd). Stashing the flag on the compiler for the modifier to read does not help, because the modifier ran first. The value reaching PrintExpressionCompiler confirms it: for {$var|escape nofilter} it is already just $_smarty_tpl->getValue('var') (the escape is gone), with an empty modifierlist.

So the information would have to travel backward in time (known late, at the tag's option flags, but needed early, when the modifier compiles). Making escape see nofilter would require routing modifiers through the post-attribute path or a two-pass compile, which is a much bigger change.

The fix that does work

Have escape:'html' emit htmlspecialchars(...) and call $compiler->setRawOutput(true) instead of the no-op. This is the mechanism htmlall/url already use, and it works because the information flows in the natural direction: the modifier sets a flag at compile time, and PrintExpressionCompiler reads it later (the !isRawOutput() check). The explicit escape then always escapes, and the outer auto-escaping skips itself because the output is already marked safe. One mechanism covers both the nofilter case and the |raw case, without escape needing to know about either. force keeps its forced double-escape semantics by not setting raw output.

With that fix, {$var|escape|nl2br} compiles to nl2br(htmlspecialchars($var)) and is correct with or without nofilter.

Workarounds on 5.4.0 today

All of these produce escaped content plus a real <br />, because they emit the escaping inline and suppress the outer auto-escaping:

• {$description|escape:'htmlall'|nl2br} (htmlall already sets raw output, so it is not subject to the no-op)
• {$description|escape:'force'|nl2br nofilter}
• {$description|escape:'force'|nl2br|raw}

Optional follow-up

nl2br is a special case because it emits HTML, which auto-escaping is not designed to handle (modifiers normally transform text; producing markup is the job of tags). Making {$var|nl2br} alone produce escaped content plus real breaks would mean having nl2br escape its own input and mark its output safe, like Twig's pre_escape + is_safe, gated on !isRawOutput() so it does not double-escape an explicit {$var|escape|nl2br}. That is a behavior change, so it fits a minor release rather than a fix.

@wisskid, if the setRawOutput approach works for you, I'll open a PR with the fix, unit tests for the nofilter + explicit escape combination, and a docs pass to restore the nofilter documentation.

[wisskid]

@Amaury sounds good, go for it! Let me know if you need any help.

[Amaury]

The PR is ready: #1204

Apologies for the delay. Doing this properly turned out to be more involved than expected. The fix itself is the setRawOutput approach described above, but while testing it I found that the raw output marker could leak between tags: when a marker-setting modifier is compiled inside a tag that prints nothing (e.g. {if $x|escape:'url'} or {assign var=y value=$x|raw}), the marker was never reset and disabled auto-escaping of the next printed variable. That leak already exists in 5.4.0 with url/htmlall/|raw, and would have widened significantly with this fix since plain |escape now sets the marker too. So the PR also hardens the marker lifecycle: it is now discarded at the end of every tag compilation, with dedicated regression tests for each leak scenario.

The PR includes the unit tests for the nofilter + explicit escape combination, and restores the nofilter documentation in the auto-escaping section (plus documents the force mode, as noted in the report).

@turtlexiv with this fix your original pattern {$description|escape|nl2br nofilter} works as expected again (escaped content with real <br /> tags), and it even works without nofilter now. Thanks again for the report.