CVE-2020-28036: Missing Authorization (CWE-862)
value: wp-includes/class-wp-xmlrpc-server.php in WordPress before 5.5.2 allows attackers to gain privileges by using XML-RPC to comment on a post.
CWE description: The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 9.8
Published: 2020-11-02T21:15Z
Repository: https://github.com/WordPress/wordpress-develop
Language: PHP
Files changed: src/wp-includes/class-wp-xmlrpc-server.php, tests/phpunit/tests/xmlrpc/wp/newComment.php
Commit date: 2020-10-29 17:42:13 +0000
Commit message: XML-RPC: Improve error messages for unprivileged users.

Add specific permission checks to avoid ambiguous failure messages.

Props zieladam, peterwilsoncc, xknown, whyisjake.

git-svn-id: https://develop.svn.wordpress.org/trunk@49380 602fd350-edb4-49c9-b593-d223f7449a82

Diff: diff --git a/src/wp-includes/class-wp-xmlrpc-server.php b/src/wp-includes/class-wp-xmlrpc-server.php
index ae6bdd0cb6..53566cbeff 100644
--- a/src/wp-includes/class-wp-xmlrpc-server.php
+++ b/src/wp-includes/class-wp-xmlrpc-server.php
@@ -3874,10 +3874,25 @@ class wp_xmlrpc_server extends IXR_Server {
 
 		if ( ! comments_open( $post_id ) ) {
 			return new IXR_Error( 403, __( 'Sorry, comments are closed for this item.' ) );
 		}
 
+		if (
+			'publish' === get_post_status( $post_id ) &&
+			! current_user_can( 'edit_post', $post_id ) &&
+			post_password_required( $post_id )
+		) {
+			return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
+		}
+
+		if (
+			'private' === get_post_status( $post_id ) &&
+			! current_user_can( 'read_post', $post_id )
+		) {
+			return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
+		}
+
 		$comment = array(
 			'comment_post_ID' => $post_id,
 			'comment_content' => trim( $content_struct['content'] ),
 		);
 
diff --git a/tests/phpunit/tests/xmlrpc/wp/newComment.php b/tests/phpunit/tests/xmlrpc/wp/newComment.php
index 8cb90fe61c..08668cb320 100644
--- a/tests/phpunit/tests/xmlrpc/wp/newComment.php
+++ b/tests/phpunit/tests/xmlrpc/wp/newComment.php
@@ -4,28 +4,58 @@
  * @group xmlrpc
  */
 class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 
 	/**
-	 * Post object for shared fixture.
+	 * Array of posts.
 	 *
-	 * @var WP_Post
+	 * @var WP_Post[]
 	 */
-	public static $post;
+	public static $posts;
+
+	/**
+	 * User IDs.
+	 *
+	 * Array of user IDs keyed by role.
+	 *
+	 * @var int[]
+	 */
+	public static $user_ids;
 
 	public static function wpSetUpBeforeClass( $factory ) {
-		self::make_user_by_role( 'administrator' );
-		self::$post = $factory->post->create_and_get();
+		self::$user_ids                     = array(
+			'administrator' => self::make_user_by_role( 'administrator' ),
+			'contributor'   => self::make_user_by_role( 'contributor' ),
+		);
+		self::$posts['publish']             = $factory->post->create_and_get();
+		self::$posts['password']            = $factory->post->create_and_get(
+			array(
+				'post_password' => 'xmlrpc',
+				'post_author'   => self::$user_ids['administrator'],
+			)
+		);
+		self::$posts['private']             = $factory->post->create_and_get(
+			array(
+				'post_status' => 'private',
+				'post_author' => self::$user_ids['administrator'],
+			)
+		);
+		self::$posts['private_contributor'] = $factory->post->create_and_get(
+			array(
+				'post_status' => 'private',
+				'post_author' => self::$user_ids['contributor'],
+			)
+		);
 	}
 
 	function test_valid_comment() {
 		$result = $this->myxmlrpcserver->wp_newComment(
 			array(
 				1,
 				'administrator',
 				'administrator',
-				self::$post->ID,
+				self::$posts['publish']->ID,
 				array(
 					'content' => rand_str( 100 ),
 				),
 			)
 		);
@@ -37,11 +67,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 		$result = $this->myxmlrpcserver->wp_newComment(
 			array(
 				1,
 				'administrator',
 				'administrator',
-				self::$post->ID,
+				self::$posts['publish']->ID,
 				array(
 					'content' => '',
 				),
 			)
 		);
@@ -57,11 +87,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 		$result = $this->myxmlrpcserver->wp_newComment(
 			array(
 				1,
 				'administrator',
 				'administrator',
-				self::$post->ID,
+				self::$posts['publish']->ID,
 				array(
 					'content' => '   ',
 				),
 			)
 		);
@@ -77,11 +107,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 		$result = $this->myxmlrpcserver->wp_newComment(
 			array(
 				1,
 				'administrator',
 				'administrator',
-				self::$post->ID,
+				self::$posts['publish']->ID,
 				array(
 					'content' => '0',
 				),
 			)
 		);
@@ -97,11 +127,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 		$result = $this->myxmlrpcserver->wp_newComment(
 			array(
 				1,
 				'administrator',
 				'administrator',
-				self::$post->ID,
+				self::$posts['publish']->ID,
 				array(
 					'content' => '   ',
 				),
 			)
 		);
@@ -137,11 +167,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 	function test_new_comment_duplicated() {
 		$comment_args = array(
 			1,
 			'administrator',
 			'administrator',
-			self::$post->ID,
+			self::$posts['publish']->ID,
 			array(
 				'content' => rand_str( 100 ),
 			),
 		);
 
@@ -166,11 +196,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 
 		$comment_args = array(
 			1,
 			'',
 			'',
-			self::$post->ID,
+			self::$posts['publish']->ID,
 			array(
 				'author'       => 'WordPress',
 				'author_email' => 'noreply@wordpress.org',
 				'content'      => 'Test Anon Comments',
 			),
@@ -191,11 +221,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 
 		$comment_args = array(
 			1,
 			'',
 			'',
-			self::$post->ID,
+			self::$posts['publish']->ID,
 			array(
 				'author'       => 'WordPress',
 				'author_email' => 'noreply at wordpress.org',
 				'content'      => 'Test Anon Comments',
 			),
@@ -216,11 +246,11 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 
 		$comment_args = array(
 			1,
 			'administrator',
 			'administrator',
-			self::$post->ID,
+			self::$posts['publish']->ID,
 			array(
 				'author'       => 'WordPress',
 				'author_email' => 'noreply at wordpress.org',
 				'content'      => 'Test Anon Comments',
 			),
@@ -230,6 +260,133 @@ class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {
 		$comment = get_comment( $result );
 		$user_id = get_user_by( 'login', 'administrator' )->ID;
 
 		$this->assertSame( $user_id, (int) $comment->user_id );
 	}
+
+	/**
+	 * Ensure users can only comment on posts they're permitted to access.
+	 *
+	 * @dataProvider data_comments_observe_post_permissions
+	 *
+	 * @param string $post_key      Post identifier from the self::$posts array.
+	 * @param string $username      Username leaving comment.
+	 * @param bool   $expected      Expected result. True: successfull comment. False: Refused comment.
+	 * @param string $anon_callback Optional. Allow anonymous comment callback. Default __return_false.
+	 */
+	function test_comments_observe_post_permissions( $post_key, $username, $expected, $anon_callback = '__return_false' ) {
+		add_filter( 'xmlrpc_allow_anonymous_comments', $anon_callback );
+
+		$comment_args = array(
+			1,
+			$username,
+			$username,
+			self::$posts[ $post_key ]->ID,
+			array(
+				'author'       => 'WordPress',
+				'author_email' => 'noreply@wordpress.org',
+				'content'      => 'Test Comment',
+			),
+		);
+
+		$result = $this->myxmlrpcserver->wp_newComment( $comment_args );
+		if ( $expected ) {
+			$this->assertInternalType( 'int', $result );
+			return;
+		}
+
+		$this->assertIXRError( $result );
+		$this->assertSame( 403, $result->code );
+	}
+
+	/**
+	 * Data provider for test_comments_observe_post_permissions.
+	 *
+	 * @return array[] {
+	 *     @type string Post identifier from the self::$posts array.
+	 *     @type string Username leaving comment.
+	 *     @type bool   Expected result. True: successfull comment. False: Refused comment.
+	 *     @type string Optional. Allow anonymous comment callback. Default __return_false.
+	 * }
+	 */
+	function data_comments_observe_post_permissions() {
+		return array(
+			// 0: Post author, password protected public post.
+			array(
+				'password',
+				'administrator',
+				true,
+			),
+			// 1: Low privileged non-author, password protected public post.
+			array(
+				'password',
+				'contributor',
+				false,
+			),
+			// 2: Anonymous user, password protected public post.
+			array(
+				'password',
+				'', // Anonymous user.
+				false,
+			),
+			// 3: Anonymous user, anon comments allowed, password protected public post.
+			array(
+				'password',
+				'', // Anonymous user.
+				false,
+				'__return_true',
+			),
+
+			// 4: Post author, private post.
+			array(
+				'private',
+				'administrator',
+				true,
+			),
+			// 5: Low privileged non-author, private post.
+			array(
+				'private',
+				'contributor',
+				false,
+			),
+			// 6: Anonymous user, private post.
+			array(
+				'private',
+				'', // Anonymous user.
+				false,
+			),
+			// 7: Anonymous user, anon comments allowed, private post.
+			array(
+				'private',
+				'', // Anonymous user.
+				false,
+				'__return_true',
+			),
+
+			// 8: High privileged non-author, private post.
+			array(
+				'private_contributor',
+				'administrator',
+				true,
+			),
+			// 9: Low privileged author, private post.
+			array(
+				'private_contributor',
+				'contributor',
+				true,
+			),
+			// 10: Anonymous user, private post.
+			array(
+				'private_contributor',
+				'', // Anonymous user.
+				false,
+			),
+			// 11: Anonymous user, anon comments allowed, private post.
+			array(
+				'private_contributor',
+				'', // Anonymous user.
+				false,
+				'__return_true',
+			),
+		);
+	}
 }

Vulnerable code: 	 * Post object for shared fixture.
	 * @var WP_Post
	public static $post;
		self::make_user_by_role( 'administrator' );
		self::$post = $factory->post->create_and_get();
				self::$post->ID,
				self::$post->ID,
				self::$post->ID,
				self::$post->ID,
				self::$post->ID,
			self::$post->ID,
			self::$post->ID,
			self::$post->ID,
			self::$post->ID,
Fixed code: 		if (
			'publish' === get_post_status( $post_id ) &&
			! current_user_can( 'edit_post', $post_id ) &&
			post_password_required( $post_id )
		) {
			return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
		}

		if (
			'private' === get_post_status( $post_id ) &&
			! current_user_can( 'read_post', $post_id )
		) {
			return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
		}

	 * Array of posts.
	 * @var WP_Post[]
	public static $posts;

	/**
	 * User IDs.
	 *
	 * Array of user IDs keyed by role.
	 *
	 * @var int[]
	 */
	public static $user_ids;
		self::$user_ids                     = array(
			'administrator' => self::make_user_by_role( 'administrator' ),
			'contributor'   => self::make_user_by_role( 'contributor' ),
		);
		self::$posts['publish']             = $factory->post->create_and_get();
		self::$posts['password']            = $factory->post->create_and_get(
			array(
				'post_password' => 'xmlrpc',
				'post_author'   => self::$user_ids['administrator'],
			)
		);
		self::$posts['private']             = $factory->post->create_and_get(
			array(
				'post_status' => 'private',
				'post_author' => self::$user_ids['administrator'],
			)
		);
		self::$posts['private_contributor'] = $factory->post->create_and_get(
			array(
				'post_status' => 'private',
				'post_author' => self::$user_ids['contributor'],
			)
		);
				self::$posts['publish']->ID,
				self::$posts['publish']->ID,
				self::$posts['publish']->ID,
				self::$posts['publish']->ID,
				self::$posts['publish']->ID,
			self::$posts['publish']->ID,
			self::$posts['publish']->ID,
			self::$posts['publish']->ID,
			self::$posts['publish']->ID,

	/**
	 * Ensure users can only comment on posts they're permitted to access.
	 *
	 * @dataProvider data_comments_observe_post_permissions
	 *
	 * @param string $post_key      Post identifier from the self::$posts array.
	 * @param string $username      Username leaving comment.
	 * @param bool   $expected      Expected result. True: successfull comment. False: Refused comment.
	 * @param string $anon_callback Optional. Allow anonymous comment callback. Default __return_false.
	 */
	function test_comments_observe_post_permissions( $post_key, $username, $expected, $anon_callback = '__return_false' ) {
		add_filter( 'xmlrpc_allow_anonymous_comments', $anon_callback );

		$comment_args = array(
			1,
			$username,
			$username,
			self::$posts[ $post_key ]->ID,
			array(
				'author'       => 'WordPress',
				'author_email' => 'noreply@wordpress.org',
				'content'      => 'Test Comment',
			),
		);

		$result = $this->myxmlrpcserver->wp_newComment( $comment_args );
		if ( $expected ) {
			$this->assertInternalType( 'int', $result );
			return;
		}

		$this->assertIXRError( $result );
		$this->assertSame( 403, $result->code );
	}

	/**
	 * Data provider for test_comments_observe_post_permissions.
	 *
	 * @return array[] {
	 *     @type string Post identifier from the self::$posts array.
	 *     @type string Username leaving comment.
	 *     @type bool   Expected result. True: successfull comment. False: Refused comment.
	 *     @type string Optional. Allow anonymous comment callback. Default __return_false.
	 * }
	 */
	function data_comments_observe_post_permissions() {
		return array(
			// 0: Post author, password protected public post.
			array(
				'password',
				'administrator',
				true,
			),
			// 1: Low privileged non-author, password protected public post.
			array(
				'password',
				'contributor',
				false,
			),
			// 2: Anonymous user, password protected public post.
			array(
				'password',
				'', // Anonymous user.
				false,
			),
			// 3: Anonymous user, anon comments allowed, password protected public post.
			array(
				'password',
				'', // Anonymous user.
				false,
				'__return_true',
			),

			// 4: Post author, private post.
			array(
				'private',
				'administrator',
				true,
			),
			// 5: Low privileged non-author, private post.
			array(
				'private',
				'contributor',
				false,
			),
			// 6: Anonymous user, private post.
			array(
				'private',
				'', // Anonymous user.
				false,
			),
			// 7: Anonymous user, anon comments allowed, private post.
			array(
				'private',
				'', // Anonymous user.
				false,
				'__return_true',
			),

			// 8: High privileged non-author, private post.
			array(
				'private_contributor',
				'administrator',
				true,
			),
			// 9: Low privileged author, private post.
			array(
				'private_contributor',
				'contributor',
				true,
			),
			// 10: Anonymous user, private post.
			array(
				'private_contributor',
				'', // Anonymous user.
				false,
			),
			// 11: Anonymous user, anon comments allowed, private post.
			array(
				'private_contributor',
				'', // Anonymous user.
				false,
				'__return_true',
			),
		);
	}