WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

Detect failures to use esc_attr() for printing in HTML attributes

Open westonruter opened this issue 9 years ago • 6 comments

The WordPress.XSS.EscapeOutput sniff will check if unescaped data is output. However, it does not distinguish between output contexts, meaning that the following is not flagged as a error when it obviously should be:

<?php $value = 'end of attribute" onclick="evil()'; ?>
<input type=button value="<?php echo esc_html( $value ) ?>">

The sniff should be updated to help guard against an HTML attribute injection vulnerability.

I suggest that the sniff be updated to look for instances of non-attribute escaping functions, and if the printing function was immediately preceded by a PHP open tag, which in turn is immediately preceded by a string that matches /\w+=['"]$/, that an error should be raised to indicate that esc_attr() should be used.

westonruter avatar Feb 08 '16 21:02 westonruter

+1 I've thought of this before but never opened an issue for it.

JDGrimes avatar Feb 08 '16 21:02 JDGrimes

Makes sense.

It won't catch every case, as in non-XML serialisation of HTML, quotes around attribute values are optional for values without spaces. /\w+=['"]?$/would be needed to be satisfied.

One other point to consider though - some attribute values, such as img src, or a href should be escaped with esc_url(), so you'd need to check for those attribute names too.

GaryJones avatar Feb 08 '16 22:02 GaryJones

One other point to consider though - some attribute values, such as img src, or a href should be escaped with esc_url(), so you'd need to check for those attribute names too.

+1

danielbachhuber avatar Nov 21 '17 15:11 danielbachhuber

FWIW, I think esc_html() and esc_attr() are currently equivalent (I would have to double-check though).

Also, similar to what @GaryJones pointed out, special consideration should probably be given to the style attribute as well.

JDGrimes avatar Dec 07 '17 14:12 JDGrimes

They are equivalent, except that the former applies esc_html filters whereas the latter applies attribute_escape filters.

westonruter avatar Dec 07 '17 20:12 westonruter

Hello, any update on how to resolve this? I am facing the same issue. I've been able to inject javascript code as the value of an HTML element attribute such as "oninput" and execute it when a user starts typing in an input box.

efrodriguez avatar Jun 10 '20 12:06 efrodriguez