Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#43141 closed defect (bug) (invalid)

wp_nonce_url() in combination with sprintf %s

Reported by: qvarting's profile qvarting Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9.2
Component: Formatting Keywords:
Focuses: Cc:

Description

Hi,

The following code does not output the correct data:

<?php
echo sprintf(
        '<a href="' . esc_url( wp_nonce_url( admin_url('admin-post.php') . '?action=%s&item_id=123' ) ) . '">%s</a>',
        $enabled ? 'action_disable' : 'action_enable',
        $enabled ? 'Disable item' : 'Enable item'
);

Result:
/wp-admin/admin-post.php?action=%20%20%20%20%20%20%20%20%20%20%20action_disable&item_id=123&_wpnonce=05bd1b0af4

Note all spaces after action=

Cheers

Change History (2)

#1 @johnbillion
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Thanks for the report @qvarting, and welcome to WordPress Trac.

This is a problem caused by the order in which you're constructing your URL. (In addition, you're not passing a valid action parameter to the wp_nonce_url() function, which means it's not properly protected.)

The %s value for your action parameter is being passed through wp_nonce_url(), which in turn passes it through add_query_arg(). Inside this function, query variables get URL-encoded so they are correctly displayed. A percent % symbol is not valid in a query variable as-is, so it gets URL-encoded as %25. This means the value for your action parameter becomes %25s. This gets treated by sprintf() as a right-justified string with length of 25. Hence, you get 12 spaces prepended to the value as padding.

The correct way to construct the URL is to perform your placeholder replacement before passing it through any function which deals with reformatting the URL. A query variable containing a non-encoded % symbol will get URL-encoded if you pass it through various functions in WordPress or PHP.

$action = ( $enabled ) ? 'action_disable' : 'action_enable';
$url = add_query_arg( array(
	'action'  => $action,
	'item_id' => 123,
), admin_url( 'admin-post.php' ) );
$url = wp_nonce_url( $url, $action );

echo sprintf(
	'<a href="%s">%s</a>',
	esc_url( $url ),
	( $enabled ) ? 'Disable item' : 'Enable item'
);

#2 @qvarting
6 years ago

Great reply @johnbillion, thanks!

Sorry for the invalid report..

Note: See TracTickets for help on using tickets.