XSS: Functions that already are escaped

[grappler]

Split off from #744

Expected next thing to be an escaping function (see Codex for 'Data Validation'), not 'get_bloginfo' Escaping of bloginfo via a filter.

get_bloginfo( 'name' ) should give an error but get_bloginfo( 'name', 'display' ) should not.

Related: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/147#issuecomment-31956920 https://github.com/Automattic/_s/pull/555

[designseer]

get_bloginfo( 'name', 'display' ) also gives the same error.

[jrfnl]

I've been looking at this, but this is not that straight-forward.

The function runs either of two filter hooks - bloginfo_url or bloginfo. https://github.com/WordPress/wordpress-develop/blob/bea4307daa21a3f97d159898b0ac676332e0e7de/src/wp-includes/general-template.php#L914-L936

For bloginfo, I can find an escaping function hooked into the filter in the default_filters.php file. For bloginfo_url, I cannot, for the life of me, find any functions hook in, let alone an escaping function.

If anyone can point to the code which hooks in an escaping function for bloginfo_url, this can be actioned, but until that time, I find this a risky change.

[SergeyBiryukov]

Some history here:

• r2935 / #1545 added a conditional to avoid applying texturize filters to the get_bloginfo( 'url' ) value.
• r3117 / #1890 added directory to the list.
• r3681 / #2381 added home to the list.
• r4168 / #2643 added the bloginfo_url filter.
• r5998 / #4516 added the display context.

This was previously brought up in #26803 get_bloginfo() doesn't sanitize URLs, even when $filter is 'display', which is currently closed as wontfix due to backward compatibility concerns, see also the discussion in #16408:comment:1.

It might be worth revisiting that ticket and applying sanitize_url() to the bloginfo_url filter.

[jrfnl]

Thank you @SergeyBiryukov for providing that context!

Based on this info, I think bloginfo( $name, 'display' ) can not always be seen as escaped and would still need additional special casing based on $name and the sniff would need to maintain a list of which keys apply the bloginfo_url filter, which makes this a more complex change and more fiddly maintenance-wise.

[jrfnl]

As a side-note: yes, I think it would be good if this is revisited in Core.