onclick attribute is not properly escaped in the _render_item method of WP_Admin_Bar class.

[tmatsuur]

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( &quot;wp&quot; )" onclick='alert( &quot;wp&quot; )'>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.

[whyisjake]

Thanks for adding this ticket @tmatsuur

[tmatsuur]

Replying to whyisjake:

Thanks @whyisjake

[dinhtungdu]

48117.diff​ escapes onclick attribute properly and prevents duplicated rendering of onclick when the href attribute presents.

[whyisjake]

In 46734:

Toolbar: Properly escape the onclick attribute.

The onclick attribute was being escaped twice, once with esc_js and again with esc_attr.

Fixes #48117.
Props tmatsuur, dinhtungdu.

Merged into WP Core in https://core.trac.wordpress.org/changeset/46734