Do Not Allow Negative IDs in wp_set_auth_cookie()
|Reported by:||mordauk||Owned by:||ryan|
|Cc:||chriscct7@…, dre@…, ratilal.sunny@…|
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.