[wp-trac] [WordPress Trac] #20771: esc_url() instead of esc_html() in wp_nonce_url()

[WordPress Trac]

#20771: esc_url() instead of esc_html() in wp_nonce_url()
-------------------------------------------------+-------------------------
 Reporter:  jkudish                              |       Owner:
     Type:  enhancement                          |  SergeyBiryukov
 Priority:  normal                               |      Status:  reopened
Component:  Formatting                           |   Milestone:  3.6
 Severity:  normal                               |     Version:  3.4
 Keywords:  has-patch dev-feedback 3.6-early     |  Resolution:
  commit                                         |
-------------------------------------------------+-------------------------
Changes (by johnbillion):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 This change has introduced an encoding bug, albeit an indirect one.

 `esc_html()` encodes ampersands as `&` but `esc_url()` encodes
 ampersands as `&`. Passing a URL through `add_query_arg()` will
 mangle the URL if it contains ampersands encoded as '&'.

 My [http://wordpress.org/extend/plugins/user-switching/ User Switching]
 plugin has broken since this change because the plugin uses
 `add_query_arg()` on a URL that's already been passed through
 `wp_nonce_url()`.

 Example:
 {{{
 $url = add_query_arg( 'action', 'foo', wp_login_url() );
 $url = wp_nonce_url( $url, 'foo' );
 $url = add_query_arg( 'redirect_to', 'bar', $url );
 }}}

 The resulting URL will end up as:
 {{{
 http://example.com/wp-
 login.php?action=foo&redirect_to=bar#038;_wpnonce=abc123
 }}}
 instead of:
 {{{
 http://example.com/wp-
 login.php?action=foo&_wpnonce=abc123&redirect_to=bar"
 }}}
 Notice that the `_wpnonce` parameter is stripped and treated as a URL hash
 instead of a query arg.

 I'd like to recommend for now that r23411 is reverted. However, the core
 cause of the problem is due to the fact that `wp_nonce_url()` should be
 sanitising the URL but not encoding it. This means it should use
 `esc_url_raw()` instead of `esc_url()` or `esc_html()`.

 I'm going to open another ticket later today to address the core issue as
 it's present in several places but generally goes unnoticed.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/20771#comment:8>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software