Make WordPress Core

Opened 9 years ago

Last modified 7 days ago

#43010 new enhancement

Attribute Name Escape

Reported by: joe_bopper Owned by:
Priority: normal Milestone: Awaiting Review
Component: Formatting Version:
Severity: normal Keywords: needs-testing needs-unit-tests has-patch
Cc: Focuses:

Description

The HTML5 spec allows us to arbitrarily named attributes for tags, e.g. data-my-arb-attr-name="attr value". This allows for generated attribute names and thus, a need to escape to avoid potential security implications.

I have seen several occasions of developers using esc_attr to resolve this case, however this is far from correct - the requirements of the name of an attribute are very different to that of the value, the best example of this simply being whitespace.

The requirements of an attribute name can be found here: https://html.spec.whatwg.org/multipage/syntax.html#attributes-2

There is a need for an esc_attr_name function to avoid compromises in html.

I have provided a simple addition patch to wp-includes/formatting.php which should resolve this issue.

Attachments (2)

esc-attr-name.diff (1.1 KB ) - added by joe_bopper 9 years ago.
EscAttrName.php (974 bytes ) - added by joe_bopper 9 years ago.
Unit test

Download all attachments as: .zip

Change History (13)

#1 @joe_bopper
9 years ago

  • Keywords has-patch needs-unit-tests added

#2 @joe_bopper
9 years ago

  • Focuses template coding-standards added

#3 @swissspidy
9 years ago

  • Component FormattingGeneral
  • Focuses template coding-standards removed

@joe_bopper
9 years ago

Unit test

#4 @joe_bopper
9 years ago

  • Keywords needs-testing added; needs-unit-tests removed

@swissspidy Cheers for having a look. For future reference, why would this be a General component and not Formatting?

#5 @swissspidy
9 years ago

  • Component GeneralFormatting

Whoops, guess I've changed the component by accident. Formatting is correct. Sorry about the confusion :-) And thanks for the tests!

#6 @joe_bopper
9 years ago

No problem. :)

#7 @sc0ttkclark
5 years ago

Finding myself needing this as I dive into building form fields for WP again. Maybe we can get an eye on this for after WP 5.9 goes out. I'll try and return and get a PR w/ test set up for this to see if helps it progress forward.

#8 @sc0ttkclark
5 years ago

Should this function use dash or underscore? Right now it's using underscore and it feels like dash may be more consistent with the data-* standard.

I also was doing some checking and there are multiple areas in WP core which output attribute names without any escaping.

You can find a number of cases in the WP core files by searching for: . '="'

You'll find lots of usage like this:

$attr_strings[] = $k . '="' . esc_attr( $v ) . '"';

#9 @sc0ttkclark
5 years ago

Should the pattern match be something more like: [^a-zA-Z0-9\-_\[\]]+

This may be more comprehensive to only allow certain character ranges versus just replacing a few characters.

#10 @swissspidy
5 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch removed

Related: wp_kses_one_attr and wp_kses_attr_check also have some logic to sanitize (not escape) attribute names based on the KSES allowlist.

This ticket was mentioned in PR #12915 on WordPress/wordpress-develop by @sainathpoojary.


7 days ago
#11

  • Keywords has-patch added; needs-patch removed

The HTML5 spec allows arbitrarily named attributes (e.g., data-*), but WordPress lacked a dedicated function to escape dynamically generated attribute names. Developers sometimes incorrectly used esc_attr() for this, which only escapes attribute values.

This PR introduces esc_attr_name() in formatting.php to resolve this. It uses a strict allowlist regex ([a-zA-Z0-9_.:\[\]-]+/u) to strip any characters not explicitly permitted in an HTML attribute name per the HTML5 specification (such as spaces, quotes, control characters, <>, =, etc.).

Trac ticket: #43010

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Note: See TracTickets for help on using tickets.