WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 5 weeks ago

#43723 new defect (bug)

Sanitize user_contactmethods output

Reported by: BjornW Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Administration Keywords: has-patch 2nd-opinion dev-feedback
Focuses: administration Cc:

Description

Data supplied in an array to the user-edit.php page via the filter 'user_contactmethods' is not properly escaped when it is outputted.

As you can see in user-edit.php the values of the $name and $desc variables are directly echoed using echo.

I'd expect it to use the WordPress Core esc_attr() as the data is used part of an html tag's attribute and therefor should be limited to what is allowed inside an html attribute.

Attachments (3)

43723.patch (1002 bytes) - added by BjornW 2 months ago.
Use esc_attr() instead of echo patch
poc-43723.php (809 bytes) - added by BjornW 2 months ago.
PoC plugin demonstrates the inproperly escaped user_contactmethods data. After applying the patch the PoC will not work anymore and thus the issue is solved in user-edit.php
43723_sanitize_html_class.patch (1015 bytes) - added by BjornW 2 months ago.
Using sanitize_html_class() to make sure the sanitized data adheres to characters allowed in a html class

Download all attachments as: .zip

Change History (12)

@BjornW
2 months ago

Use esc_attr() instead of echo patch

@BjornW
2 months ago

PoC plugin demonstrates the inproperly escaped user_contactmethods data. After applying the patch the PoC will not work anymore and thus the issue is solved in user-edit.php

#1 @BjornW
2 months ago

  • Focuses administration added
  • Keywords has-patch 2nd-opinion dev-feedback added

This ticket was mentioned in Slack in #core by bjornw. View the logs.


2 months ago

This ticket was mentioned in Slack in #core by bjornw. View the logs.


2 months ago

#4 follow-up: @joyously
2 months ago

At least for the class name, not all values that are valid for an attribute are valid for a class name. And it looks like the filter name has the unmodified $name variable in it?

#5 in reply to: ↑ 4 @BjornW
2 months ago

Replying to joyously:

At least for the class name, not all values that are valid for an attribute are valid for a class name. And it looks like the filter name has the unmodified $name variable in it?

Is there an escape function which only allows that what is allowed for a class name? If not, my guess is the esc_attr() is the best we have for now.

And yes, the filter name currently has the unmodified $name in it, which should be probably be escaped as well, but might have unwanted side-effects so my current patch does not touch this (yet).

#6 follow-up: @joyously
2 months ago

Yes, there is sanitize_html_class().

@BjornW
2 months ago

Using sanitize_html_class() to make sure the sanitized data adheres to characters allowed in a html class

#7 in reply to: ↑ 6 @BjornW
2 months ago

Replying to joyously:

Yes, there is sanitize_html_class().

Thanks, using sanitize_htmnl_class() for sanitizing the data specifically for use in an html class in my new patch 43723_sanitize_html_class.

This ticket was mentioned in Slack in #core by bjornw. View the logs.


2 months ago

This ticket was mentioned in Slack in #core by bjornw. View the logs.


5 weeks ago

Note: See TracTickets for help on using tickets.