[wp-trac] [WordPress Trac] #23480: Do Not Allow Negative IDs in wp_set_auth_cookie()

[WordPress Trac]

#23480: Do Not Allow Negative IDs in wp_set_auth_cookie()
-----------------------------+--------------------------
 Reporter:  mordauk          |       Type:  defect (bug)
   Status:  new              |   Priority:  normal
Milestone:  Awaiting Review  |  Component:  Users
  Version:  3.5.1            |   Severity:  major
 Keywords:  has-patch        |
-----------------------------+--------------------------
 After discovering a flaw in a plugin that made it possible for users to
 bypass the plugin's login form /  user validation, I've found an issue
 with wp_set_auth_cookie() that I believe to be critical.

 Since all user validation is (and should be) done prior to calling
 `wp_set_auth_cookie`, the function allows any user ID to be passed to it
 and the cookie will be set.

 The user ID passed to `wp_set_auth_cookie` gets passed to
 `wp_generate_auth_cookie`, which then retrieves the user data from the ID
 with `get_userdata`, which goes through `get_user_by` and
 `WP_User::get_data_by`. The `WP_User::get_data_by` method runs `absint` on
 the original user ID passed to `wp_set_auth_cookie`.

 `absint` will translate any negative number to its positive counter part.

 The problem with this is that `wp_set_auth_cookie` will happily generate a
 valid cookie even when given an invalid user ID, such as -1.

 While it is the responsibility of plugins to ensure `wp_set_auth_cookie`
 never gets called except when all user data has been fully validated, it
 seems crazy to me that the function will still successfully generate the
 auth cookies when given a negative user ID.

 In the case of the flaw discovered in the plugin mentioned above, -1 was
 being used to indicate invalid user data (which seems fine), but then the
 -1 was getting passed (when it shouldn't have been) to
 `wp_set_auth_cookie`, which simply translated the user ID to 1 (almost
 always the admin account) and logged the invalid user in as an admin.
 While it is obviously a bug on the plugin's part that -1 ever  reached
 `wp_set_auth_cookie`, it still should have died gracefully.

 I'm proposing we for `wp_set_auth_cookie` to stop short if the user ID
 isn't an INT or if it is negative.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/23480>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software