Skip to content

Incorrect message for Security.ProperEscapingFunction.htmlAttrNotByEscHTML #554

@GaryJones

Description

@GaryJones

Bug Description

The message for WordPressVIPMinimum.Security.ProperEscapingFunction.htmlAttrNotByEscHTML doesn't allow for the possibility of the value being a URL, and therefore esc_url() being correct instead of esc_attr() i.e. it gives the wrong advice in some circumstances.

Minimal Code Snippet

<form method="post" action="<?php echo esc_html(admin_url('admin.php?page='.$base_name.'&amp;mode=logs&amp;id='.$poll_id)); ?>">

That gave the following message:

🚫 Error: Wrong escaping function. HTML attributes should be escaped by esc_attr(), not by esc_html() (WordPressVIPMinimum.Security.ProperEscapingFunction.htmlAttrNotByEscHTML).

Since the value is a URL, the correct escaping would be esc_url().

Error Code

WordPressVIPMinimum.Security.ProperEscapingFunction.htmlAttrNotByEscHTML

Environment

Question Answer
PHP version 7.?.?
PHP_CodeSniffer version 3.5.5
VIPCS version 2.1.0

Additional Context (optional)

In the simplest fix, the message just needs an "or esc_url()" added to it.

We could potentially check if the attribute is an src or href, or action on certain elements, or if the call contained a URL-related function (home_url(), admin_url(), etc.) to give a tighter message, but there will always be cases of data-* attributes, or simple variables where we couldn't determine for certain if the value was meant to be a URL. The simple change in message avoids the completely incorrect message for some situations.

Tested Against master branch?

  • I have verified the issue still exists in the master branch of VIPCS.
  • I have verified the issue still exists in the develop branch of VIPCS.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions