Changeset 34034

Timestamp:

09/11/2015 02:24:03 AM (9 years ago)

Author:

boonebgorges

Message:

Require numeric IDs in user deletion functions.

wp_delete_user() and wpmu_delete_user() both require an $id parameter.
Previously, the functions did not verify that the value passed was, in fact,
a number. As such, passing an object or any other entity that would be cast
to int 1 would result in user 1 being deleted. We fix this by enforcing
the requirement that $id be numeric.

Props dipesh.kakadiya, utkarshpatel, juliobox.
Fixes #33800.

Location:

trunk

Files:

• src/wp-admin/includes/ms.php (modified) (1 diff)
• src/wp-admin/includes/user.php (modified) (1 diff)
• tests/phpunit/tests/user/multisite.php (modified) (1 diff)
• tests/phpunit/tests/user/wpDeleteUser.php (modified) (1 diff)

trunk/src/wp-admin/includes/ms.php

@@ -185 +185 @@
 function wpmu_delete_user( $id ) {
     global $wpdb;
+
+    if ( ! is_numeric( $id ) ) {
+        return false;
+    }
 
     $id = (int) $id;

trunk/src/wp-admin/includes/user.php

@@ -273 +273 @@
 function wp_delete_user( $id, $reassign = null ) {
     global $wpdb;
+
+    if ( ! is_numeric( $id ) ) {
+        return false;
+    }
 
     $id = (int) $id;

trunk/tests/phpunit/tests/user/multisite.php

@@ -345 +345 @@
     }
 
+    public function test_numeric_string_user_id() {
+        $u = $this->factory->user->create();
+
+        $u_string = (string) $u;
+        $this->assertTrue( wpmu_delete_user( $u_string ) );
+        $this->assertFalse( get_user_by( 'id', $u ) );
+    }
+
+    /**
+     * @ticket 33800
+     */
+    public function test_should_return_false_for_non_numeric_string_user_id() {
+        $this->assertFalse( wpmu_delete_user( 'abcde' ) );
+    }
+
+    /**
+     * @ticket 33800
+     */
+    public function test_should_return_false_for_object_user_id() {
+        $u_obj = $this->factory->user->create_and_get();
+        $this->assertFalse( wpmu_delete_user( $u_obj ) );
+        $this->assertEquals( $u_obj->ID, username_exists( $u_obj->user_login ) );
+    }
 }
 

trunk/tests/phpunit/tests/user/wpDeleteUser.php

@@ -126 +126 @@
         $this->assertEquals( $reassign, $post->post_author );
     }
+
+    public function test_numeric_string_user_id() {
+        if ( is_multisite() ) {
+            $this->markTestSkipped( 'wp_delete_user() does not delete user records in Multisite.' );
+        }
+
+        $u = $this->factory->user->create();
+
+        $u_string = (string) $u;
+        $this->assertTrue( wp_delete_user( $u_string ) );
+        $this->assertFalse( get_user_by( 'id', $u ) );
+    }
+
+    /**
+     * @group 33800
+     */
+    public function test_should_return_false_for_non_numeric_string_user_id() {
+        $this->assertFalse( wp_delete_user( 'abcde' ) );
+    }
+
+    /**
+     * @group 33800
+     */
+    public function test_should_return_false_for_object_user_id() {
+        if ( is_multisite() ) {
+            $this->markTestSkipped( 'wp_delete_user() does not delete user records in Multisite.' );
+        }
+
+        $u_obj = $this->factory->user->create_and_get();
+        $this->assertFalse( wp_delete_user( $u_obj ) );
+        $this->assertEquals( $u_obj->ID, username_exists( $u_obj->user_login ) );
+    }
 }