Add StripTags sniff and amend rulesets and existing sniffs

[rebeccahum]

This resolves #133.

What this PR does:

• add a StripTags sniff
• remove the WordPress.AlternativeFunctions rules for strip tags
• remove strip_tags from RestrictedFunctions sniff

[GaryJones]

Background: WPCS code added a straight replacement suggestion if strip_tags() was used. wp_strip_all_tags() is recommended, as it properly strips all HTML tags, including the contents of script and style elements.

#133 says that using strip_tags( $some_variable, '<p><a><iframe><strong><img>' ); would be wrong. @david-binda is this because wp_kses() does a better job, security wise, than strip_tags()?

The first warning in this PR basically matches that from WPCS.

The second recomends wp_kses() when the second arg is used in the strip_tags() call.

cc @jrfnl in case this would be suitable for WPCS.

[jrfnl]

@GaryJones WPCS does not recommend using wp_strip_all_tags() when the second parameter is passed. See: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/46d42828ce7355d8b3776e3171f2bda892d179b4/WordPress/Sniffs/WP/AlternativeFunctionsSniff.php#L145-L154

Adding a suggestion for using wp_kses() in those cases could be an addition to the existing WPCS sniff. No new sniff needed for that.

On another note: please open issues in WPCS with these kind of questions/suggestions so they can be discussed in the proper place and other people can pitch in as well.
Unless I'm being paid for it, I'm not happy to review or even look at sniffs from a commercial company.

[GaryJones]

@jrfnl Thanks :-)

Adding a suggestion for using wp_kses() in those cases could be an addition to the existing WPCS sniff. No new sniff needed for that.

@rebeccahum Based on the above, I think it would be better see about opening a PR on WPCS for the same end result, and if successful. closing out this PR unmerged.

[jrfnl]

Based on the above, I think it would be better see about opening a PR on WPCS for the same end result

I'm not sure such a PR should be accepted to WPCS as wp_kses() is quite a different function from strip_tags(). wp_kses() is only intended for output escaping, while (wp_)strip_(all_)tags() can also be used as part of input sanitization.

[rebeccahum]

@GaryJones Should we change it where if there's a second parameter passed in, the sniff doesn't error/warning?

[GaryJones]

Should we change it where if there's a second parameter passed in, the sniff doesn't error/warning?

That's what the current WPCS code does already I believe.

https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/46d42828ce7355d8b3776e3171f2bda892d179b4/WordPress/Tests/WP/AlternativeFunctionsUnitTest.inc#L35

[david-binda]

@david-binda is this because wp_kses() does a better job, security wise, than strip_tags()?

Well, the issue is two fold. First of all, wp_strip_all_tags does strip all tags and there is no way to preserve any tags. The other difference in between strip_tags and wp_strip_all_tags is that the later removes content enclosed by <script> and <style> params:

wp> $text = '<p><strong><a href="#somelink">Link</a></strong><iframe src="https://example.com"></iframe><img src="./images/image.gif"/></p><script>alert("haxxored");</script>';
wp> strip_tags( $text, '<p><a><iframe><strong><img>' );
string(144) "<p><strong><a href="#somelink">Link</a></strong><iframe src="https://example.com"></iframe><img src="./images/image.gif"/></p>alert("haxxored");"
wp> wp_strip_all_tags( $text );
string(4) "Link"

More obvious example:

wp> $text = '<script>alert("haxxored")</script>';
string(34) "<script>alert("haxxored")</script>"
wp> strip_tags( $text );
string(17) "alert("haxxored")"
wp> wp_strip_all_tags( $text );
string(0) ""

So, in case we want to preserve some HTML in WordPress.com VIP world, we would use wp_kses which allows us to define a set of HMTL elements, including attributes, and protocols.