[wp-trac] [WordPress Trac] #48117: onclick attribute is not properly escaped in the _render_item method of WP_Admin_Bar class.

[WordPress Trac]

#48117: onclick attribute is not properly escaped in the _render_item method of
WP_Admin_Bar class.
--------------------------+-----------------------------
 Reporter:  tmatsuur      |      Owner:  whyisjake
     Type:  defect (bug)  |     Status:  assigned
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  General       |    Version:  5.2.3
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 I noticed that when the onclick attribute value is specified with the
 'meta' key of the add_menu method of $wp_admin_bar, the onclick attribute
 is output twice.

 {{{
 $wp_admin_bar->add_menu( array(
     'id'     => 'mylink',
     'title'  => 'mylink',
     'href'   => 'https://wordpress.org/',
     'meta'   => array(
         'onclick'  => 'alert( "wp" )',
     ),
 ) );
 }}}

 The rendered HTML source looks like this:

 {{{
 <li id='wp-admin-bar-mylink'><a class='ab-item'
 href='https://wordpress.org/' onclick="alert( "wp" )"
 onclick='alert( "wp" )'>mylink</a></li>
 }}}

 As a result of investigating this cause, if the href attribute was set in
 the _render_item method, the onclick attribute was output after being
 escaped with the esc_js function, and then was output after being escaped
 with another esc_attr function.

 If the href attribute is not set, the onclick attribute is output after
 being escaped by the esc_attr function.

 The vulnerable source code is as follows: class-wp-admin-bar.php: 536-551

 {{{
 if ( $has_link ) {
     $attributes = array( 'onclick', 'target', 'title', 'rel', 'lang',
 'dir' );
     echo "<a class='ab-item'$aria_attributes href='" . esc_url(
 $node->href ) . "'";
     if ( ! empty( $node->meta['onclick'] ) ) {
         echo ' onclick="' . esc_js( $node->meta['onclick'] ) . '"';
     }
 } else {
     $attributes = array( 'onclick', 'target', 'title', 'rel', 'lang',
 'dir' );
     echo '<div class="ab-item ab-empty-item"' . $aria_attributes;
 }

 foreach ( $attributes as $attribute ) {
     if ( ! empty( $node->meta[ $attribute ] ) ) {
         echo " $attribute='" . esc_attr( $node->meta[ $attribute ] ) .
 "'";
     }
 }
 }}}

 The variable $attributes always contains 'onclick', so it escapes with the
 esc_attr function and outputs the attribute value.

 I hope that the onclick attribute will be output properly.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/48117>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform