From 5b9d9c7c073bcd65411eec57deab649e17984f0c Mon Sep 17 00:00:00 2001
From: Boone Gorges <boonebgorges@git.wordpress.org>
Date: Fri, 11 Sep 2015 02:24:03 +0000
Subject: [PATCH] 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.

git-svn-id: https://develop.svn.wordpress.org/trunk@34034 602fd350-edb4-49c9-b593-d223f7449a82
---
 src/wp-admin/includes/ms.php              |  4 +++
 src/wp-admin/includes/user.php            |  4 +++
 tests/phpunit/tests/user/multisite.php    | 23 ++++++++++++++++
 tests/phpunit/tests/user/wpDeleteUser.php | 32 +++++++++++++++++++++++
 4 files changed, 63 insertions(+)

diff --git a/src/wp-admin/includes/ms.php b/src/wp-admin/includes/ms.php
index b7e4c28631..2907d24735 100644
--- a/src/wp-admin/includes/ms.php
+++ b/src/wp-admin/includes/ms.php
@@ -185,6 +185,10 @@ function wpmu_delete_blog( $blog_id, $drop = false ) {
 function wpmu_delete_user( $id ) {
 	global $wpdb;
 
+	if ( ! is_numeric( $id ) ) {
+		return false;
+	}
+
 	$id = (int) $id;
 	$user = new WP_User( $id );
 
diff --git a/src/wp-admin/includes/user.php b/src/wp-admin/includes/user.php
index 3c2e26e186..5c12d3d265 100644
--- a/src/wp-admin/includes/user.php
+++ b/src/wp-admin/includes/user.php
@@ -273,6 +273,10 @@ function get_users_drafts( $user_id ) {
 function wp_delete_user( $id, $reassign = null ) {
 	global $wpdb;
 
+	if ( ! is_numeric( $id ) ) {
+		return false;
+	}
+
 	$id = (int) $id;
 	$user = new WP_User( $id );
 
diff --git a/tests/phpunit/tests/user/multisite.php b/tests/phpunit/tests/user/multisite.php
index 0ae955aff8..67a8593ad6 100644
--- a/tests/phpunit/tests/user/multisite.php
+++ b/tests/phpunit/tests/user/multisite.php
@@ -344,6 +344,29 @@ class Tests_Multisite_User extends WP_UnitTestCase {
 		}
 	}
 
+	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 ) );
+	}
 }
 
 endif ;
diff --git a/tests/phpunit/tests/user/wpDeleteUser.php b/tests/phpunit/tests/user/wpDeleteUser.php
index d47e98c9cf..f345ac136c 100644
--- a/tests/phpunit/tests/user/wpDeleteUser.php
+++ b/tests/phpunit/tests/user/wpDeleteUser.php
@@ -125,4 +125,36 @@ class Tests_User_WpDeleteUser extends WP_UnitTestCase {
 		$post = get_post( $post_id );
 		$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 ) );
+	}
 }