[wp-trac] [WordPress Trac] #58336: Potential XSS on admin_body_class hook

[WordPress Trac]

#58336: Potential XSS on admin_body_class hook
----------------------------------------+---------------------
 Reporter:  rafiem                      |       Owner:  (none)
     Type:  defect (bug)                |      Status:  new
 Priority:  normal                      |   Milestone:  6.3
Component:  Security                    |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  needs-unit-tests has-patch  |     Focuses:
----------------------------------------+---------------------
Changes (by SergeyBiryukov):

 * keywords:  needs-patch needs-unit-tests => needs-unit-tests has-patch


Comment:

 Thanks for the comparison!

 I was thinking of a general principle of
 [https://developer.wordpress.org/apis/security/#h-guiding-principles
 sanitizing early and escaping as late as possible], so for the output
 itself, `esc_attr()` seems more appropriate.

 If I'm reading correctly, `sanitize_html_class()` does not allow spaces,
 so the suggestion from comment:3 would break if there are multiple classes
 in `$admin_body_classes`, which is the case by default.

 It is worth noting that `wp-admin/customize.php` [source:tags/6.2.1/src
 /wp-admin/customize.php?marks=184#L175 already uses esc_attr() for the
 output of body classes], so I believe we should do the same here. It would
 also resolve the XSS mentioned in the ticket description.

 With that in mind, I think [attachment:"58336.diff"] should address the
 issue raised here.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58336#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform