1

Is using get_the_term_list and get_field in the manner below consistent with accepted standards for safely displaying taxonomy terms as links in a table and custom field text beneath headings in a template file?

Are additional functions recommended that are more accepted and secure?

Or is displaying the terms or text in this manner already on par with safety standards as is?

<table>
<tbody>
<tr><td><strong>Genre:</strong></td><td><?php echo get_the_term_list( $post->ID, 'genre', ' ', ', ', '' ); ?></td></tr>
<tr><td><strong>Country:</strong></td><td><?php echo get_the_term_list( $post->ID, 'country', ' ', ', ', '' ); ?></td></tr>
</tbody>
</table>
<br/>
<h3>Quote:</h3>
<span><?php echo get_field('quote'); ?></span>
<br/>
<h3>Review:</h3>
<blockquote><?php echo get_field('review');?></blockquote>

What sparked this question was coming across https://developer.wordpress.org/themes/theme-security/data-sanitization-escaping/ specifically the part that says "esc_html() – Use this function anytime an HTML element encloses a section of data being displayed."

Improve this question

1 Answer 1

Reset to default
2

Well I typed up an answer yesterday then deleted it, hoping that someone more confident in their answer would come along. But I will at least share my understanding.

...which is, that all content being put into the page should be sanitized, and as lately as possible. This means you want to use those functions (i.e. esc_html) right when you are outputting. Because of this "as late as possible" principle, WordPress does not (Edit: always) pre-escape for you (which would increase the odds for accidental double escaping).

Edit: per our discussion in the comments and further research, get_the_term_list is one of WordPress' pre-escaped functions, and doesn't need to be further escaped. As it is escaped through the use of a filter hook, there are technically safer ways to do it at a later point (i.e. using get_the_terms and then manually outputting with proper escaping). However, the built-in escaping is safe enough that the core themes rely on it, so it's probably sufficient.

Neither does ACF sanitize output for you. In fact the only comment about it I could find from their developer was an oddly ambiguous one saying something to the effect of we don't, maybe WordPress does.

I have generally not used those functions for get_field, because the danger doesn't make intuitive sense to me when I am not accepting input from the frontend. But I have also learned that there are both hackers and security researchers smarter than me, so I try to defer to the accepted wisdom, which again is to go ahead and escape. So, I will do so now.

If you're overescaping, you'll probably notice because the data will get messed up. If you're underescaping, you won't know until your hacked. So...err on the side of too much.

That's my 2 cents.

Improve this answer
5
  • Thank you @tmdesigned , I have just checked wp-includes\category-template.php and line 1287 seems to indicate get_the_term_list is being "pre-escaped" via esc_url, would you agree? I concur all get_field instances should be escaped. In reading your answer though it seems you're saying using get_anything in a template file is not recommended, is that right? If so, may I ask what you might recommend as a more secure method for outputting data in a template? If you might know of any recommended resources for novices about this subject that would be much appreciated too.
    – movsite1
    Commented Mar 25, 2019 at 1:01
  • The URL is being url escaped, but the term name is not being escaped. I wasn't saying not to use get_field, only not to use it without escaping it. However I have read some posts about using the post meta functions instead of the ACF ones to reduce database queries.
    – tmdesigned
    Commented Mar 25, 2019 at 18:32
  • I see now @tmdesigned , where I'm stuck then is how to fully escape get_the_term_list safely since I want those <a> tags to render as links. Instead might wp_kses_post (get_the_term_list($post->ID, 'genre', ' ', ', ', '' )) do the trick for a novice like myself - protect from dangerous code while rendering those <a> tags as links?
    – movsite1
    Commented Mar 25, 2019 at 23:06
  • Good point. I think wp_kses_post would would work. You could also avoid get_the_term_list and use get_the_terms instead to more directly control the output so as to late escape. That's more work, but is what WP VIP recommends if I'm reading them correctly. It seems like a lot to say certain simple built-in functions designed for output need to be treated so cautiously. I don't know. I'd say try wp_kses, which should do it, and at worst you're being over cautious. I welcome other opinions on this.
    – tmdesigned
    Commented Mar 26, 2019 at 1:31
  • I take it back. It looks like the term name part is being escaped already, just a little more quietly through a filter. See wordpress.stackexchange.com/a/80449/28273 . He notes some caveats, such as it being safer to do what we have been talking about, but it seems get_the_terms_list is one of those "meant for output" functions that is already escaping for you and would generally be safe-ish.
    – tmdesigned
    Commented Mar 26, 2019 at 1:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.