smarty-php / smarty

Smarty is a template engine for PHP, facilitating the separation of presentation (HTML/CSS) from application logic.
Other
2.24k stars 705 forks source link

setEscapeHtml true will also escape output of functions in smarty 5 #906

Closed timmit-nl closed 10 months ago

timmit-nl commented 12 months ago

Hi,

Because of the wrong release today I had a Smarty 5.0.0-rc1 on a dev enviorment. So I start moving the smarty plugins code from 4 to 5. I noticed a new behavoir:

When registering the plugins trough registerPlugin (same as before) the output of functions are now escaped if smarty is started with setEscapeHtml(true).

As our pentesters love this setting (not), we want to leave it on. But the result of our template functions are now useless to us.

And yes we escape beforehand. But if something slips trough setEscapeHtml will save the day.

So how can we have the setEscapeHtml(true) so it will not escape functions. Or can we return in the function some var so it won't escape? (as we now only return a string)

Example:

{insertCSFR}

must insert: (smarty <5) `

But now it renders to: (smarty 5) <input type="hidden" name="_CSRF_INDEX" value="dXLFTSfGeAzDtkppYKxKLmOl" /> <input type="hidden" name="_CSRF_TOKEN" value="YXbZFOh30iXiIDdn7opFnVHDt0tNBNkA_pFR0P-t1kE=" /> and that is because it compiles to: (smarty 5) <?php echo htmlspecialchars((string) $_smarty_tpl->getSmarty()->getFunctionHandler('insertcsfrtoken')->handle(array(), $_smarty_tpl), ENT_QUOTES, 'UTF-8');?>`

instead of: (smarty < 5) <?php echo call_user_func_array( $_smarty_tpl->smarty->registered_plugins[Smarty::PLUGIN_FUNCTION]['insertCSFRToken'][0], array( array(),$_smarty_tpl ) );?>

So how can we fix this?

Thanks,

Tim

wisskid commented 12 months ago

Good catch! That change was unintentional and I think we should fix it.

wisskid commented 11 months ago

@timmit-nl the fix was easy enough, but thinking through this, I feel that the behavior of functions, block-tags and auto-escaping is under-defined. In any case, the documentation is rather vague about this. Many (but not all) of the built-in functions, such as {html_checkboxes} and {html_table} return html and are not auto-escaped in Smarty4. The same goes for the block plugins. But it feels rather arbitrary. What if your custom function or block tag produces valid HTML, but you need to auto-escape the result into a JSON string? Or vice versa?

It seems to me that function and block handlers should at least somehow indicate what they are returning, i.e. plain text, html, js, etc. That way, we would be able to apply auto-escaping when needed and refrain from it when not needed.

What do you think?

timmit-nl commented 11 months ago

Yes that could be great. Some functions should be escaped, some not.

The only thing is, how do you give the result back, with the correct type. The type is in most cases (or always) a strict string. But how to differentiate is difficult on runtime.

But maybe when you register the function you tell what is is returning and possible an extra bool to force no escaping.