[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:  SergeyBiryukov
     Type:  defect (bug)  |      Status:  reopened
 Priority:  normal        |   Milestone:  6.3
Component:  Security      |     Version:
 Severity:  normal        |  Resolution:
 Keywords:                |     Focuses:
--------------------------+-----------------------------
Changes (by johnbillion):

 * keywords:  needs-unit-tests has-patch =>
 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 Additional hardening via escaping is almost always a good idea, but this
 change was made in isolation with no consideration of the underlying
 approach that WordPress core takes to filtered values.

 * Where does responsibility lie for sanitising and escaping a filtered
 value that does not originate from untrusted content (ie. literal
 strings)? This change suggests it's the responsibility of WordPress core
 instead of plugins and themes that use filters.
 * What is the approach that WordPress core takes and commits to? Note that
 WordPress core text translations are treated as "trusted" and not escaped
 upon output, but they all pass through the `gettext` filter (and others)
 and are therefore susceptible to the same concern that was raised in this
 ticket for the `admin_body_class` filter.
 * Why is the `admin_body_class` filter a special case? Is it more likely
 than others to be filtered to include a value from untrusted input? See
 the ACF vulnerability in the ticket description for example.

 There are hundreds of instances of values in WordPress core that originate
 from trusted input but pass through a filter and then get echoed without
 escaping. A quick search for `echo $` returns nearly 1,000 results, here
 are the first few filters I found that operate in this way:

 * `login_title`
 * `login_headertext`
 * `login_message`
 * `post_updated_messages`
 * `enter_title_here`

 In addition, most instances of translations in WordPress core are not
 escaped on output but first pass through the `gettext` filter.

 So the question is, is `admin_body_class` a special case? If it needs to
 be fixed then many more filters need to be fixed too, or the project needs
 to be clear on where the responsibility for sanitising and escaping
 filtered values lies (ie. with plugin and themes).

 I don't think [55846] needs to be reverted, but we do need to discuss the
 overall approach to escaping filtered values. It it not practical for
 WordPress core to treat all filtered values as untrusted input.

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