Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
timmit-nl opened this issue Sep 14, 2023 · 3 comments · Fixed by #908
Closed

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

timmit-nl opened this issue Sep 14, 2023 · 3 comments · Fixed by #908

Comments

@timmit-nl
Copy link

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)
<input type="hidden" name="_CSRF_INDEX" value="wTPNGFfqOp9NxM4jJoy8OKmA" /> <input type="hidden" name="_CSRF_TOKEN" value="dtjnI82HmcjX-_tkIQlSKvF6bxlNyN99Qh-CQlG_AHg=" />
But now it renders to: (smarty 5)
&lt;input type=&quot;hidden&quot; name=&quot;_CSRF_INDEX&quot; value=&quot;dXLFTSfGeAzDtkppYKxKLmOl&quot; /&gt; &lt;input type=&quot;hidden&quot; name=&quot;_CSRF_TOKEN&quot; value=&quot;YXbZFOh30iXiIDdn7opFnVHDt0tNBNkA_pFR0P-t1kE=&quot; /&gt;
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
Copy link
Contributor

wisskid commented Sep 14, 2023

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

@wisskid
Copy link
Contributor

wisskid commented Sep 21, 2023

@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?

wisskid added a commit that referenced this issue Sep 21, 2023
Fixes #906
This behavior is under-defined though. This requires some clear documentation.
@timmit-nl
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants