[wp-trac] [WordPress Trac] #23190: get_user_id_from_string() is returning wrong data

[WordPress Trac]

#23190: get_user_id_from_string() is returning wrong data
-------------------------------+------------------------------
 Reporter:  godhulii_1985      |       Owner:
     Type:  enhancement        |      Status:  new
 Priority:  normal             |   Milestone:  Awaiting Review
Component:  Multisite          |     Version:  3.5
 Severity:  normal             |  Resolution:
 Keywords:  reporter-feedback  |
-------------------------------+------------------------------
Description changed by DrewAPicture:

Old description:

> '''Background'''[[BR]]
> I was developing my custom theme and used google-oauth for auto user
> login. Here, I used user's google id (not login user-id, the profile id
> which is totally numeric) so that I can identify the user later. To
> create new user I used wp_insert_user().
>
> The newly created user can update his/her initial default password and it
> will fail in the wp login process so everytime I call user_signon()
> function I call wp_update_user() to update his/her password to default
> [additionally, I disable password field in wp-admin area and that works
> for general user but as you know it is not hacker proof]
>
> '''Here begins the problem'''[[BR]]
> Lets assume google says that the oauth user's id is: 123456. So, I
> created an user with user-id: 123456. Wordpress assigned 99 to the user
> (that is www.example.com/?author=99 will redirect to this user's profile)
>
> Now, when I call get_user_id_from_string('123456'), I expect 99 but I get
> 123456. I think it is a security risk because user-id is the users's
> database primary key type id (which is 99 in this case).
>
> I looked into the core "wp-includes => ms-functions.php =>
> get_user_id_from_string()" and found this segment:
>
>         elseif ( is_numeric( $string ) ) {
>                 $user_id = $string;
>         } else {
>                 $user = get_user_by('login', $string);
>                 if ( $user )
>                         $user_id = $user->ID;
>         }
>
> Here, is_numeric() gets precendance and I do not get my desired id (99)
> as my input string (or user-login-name) was 123456 which passes
> is_numeric() function.
>
> '''Problem defination'''[[BR]]
> The developers considered wp user-login-id to be alphaneumeric (I think)
> but in the documentation it is mentioned that: "user_login     A string
> that contains the user's username for logging in. " in
> http://codex.wordpress.org/Function_Reference/wp_insert_user page. It is
> not mentioned it should be alphaneumeric or not.
>

> Right now I have solved the issue by prepending 'g' infront of the oauth
> codes so I'm using 'g123456' as user-login-id in wp_insert_user() but I
> think this issue should be considered as security risk because if there
> is no binding on wp_insert_user() with a numeric value (123456) as user-
> id then get_user_id_from_string() should also respect this choice and
> return 99 here instead of 123456, otherwise wrong user will be signed
> into in this scenario.

New description:

 '''Background'''[[BR]]
 I was developing my custom theme and used google-oauth for auto user
 login. Here, I used user's google id (not login user-id, the profile id
 which is totally numeric) so that I can identify the user later. To create
 new user I used wp_insert_user().

 The newly created user can update his/her initial default password and it
 will fail in the wp login process so everytime I call user_signon()
 function I call wp_update_user() to update his/her password to default
 [additionally, I disable password field in wp-admin area and that works
 for general user but as you know it is not hacker proof]

 '''Here begins the problem'''[[BR]]
 Lets assume google says that the oauth user's id is: 123456. So, I created
 an user with user-id: 123456. Wordpress assigned 99 to the user (that is
 www.example.com/?author=99 will redirect to this user's profile)

 Now, when I call get_user_id_from_string('123456'), I expect 99 but I get
 123456. I think it is a security risk because user-id is the users's
 database primary key type id (which is 99 in this case).

 I looked into the core "wp-includes => ms-functions.php =>
 get_user_id_from_string()" and found this segment:
 {{{
         elseif ( is_numeric( $string ) ) {
                 $user_id = $string;
         } else {
                 $user = get_user_by('login', $string);
                 if ( $user )
                         $user_id = $user->ID;
         }
 }}}
 Here, is_numeric() gets precendance and I do not get my desired id (99) as
 my input string (or user-login-name) was 123456 which passes is_numeric()
 function.

 '''Problem defination'''[[BR]]
 The developers considered wp user-login-id to be alphaneumeric (I think)
 but in the documentation it is mentioned that: "user_login     A string
 that contains the user's username for logging in. " in
 http://codex.wordpress.org/Function_Reference/wp_insert_user page. It is
 not mentioned it should be alphaneumeric or not.


 Right now I have solved the issue by prepending 'g' infront of the oauth
 codes so I'm using 'g123456' as user-login-id in wp_insert_user() but I
 think this issue should be considered as security risk because if there is
 no binding on wp_insert_user() with a numeric value (123456) as user-id
 then get_user_id_from_string() should also respect this choice and return
 99 here instead of 123456, otherwise wrong user will be signed into in
 this scenario.

--

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