From 8b165cc864132092d8064c25f7f1d5713da8531d Mon Sep 17 00:00:00 2001 From: David Cavins Date: Mon, 6 Jul 2026 14:34:50 -0500 Subject: [PATCH] Improve security of messages endpoint (0.8 branch). In messages endpoint, sanitize the incoming `user_id` parameter value to avoid user spoofing. Special thanks to trihedron who first reported this issue responsibly. Props emaralive, jjj, renato, trihedron, substitute99, j2k14a, g_r_i_n_n, pythonime, dizconnect (Sanjorn Keeratirungsan), eneednar19, bb-hunter (Mustafa Ahmed), yhalo (Yaohui Wang), Ngo Anh Duc, ekbreks, jeromewincek (Jerome Wincek), taylsec, ajaah-254, izumi_hyun, mickey_cyberkid (Michael Okyere), underdog_theori, duyytrann (Duy Tran), safe-us (Safe Us Team), miauuu, daupaul (Dau-Po Yu). --- .../class-bp-rest-messages-endpoint.php | 160 +++++++++++++----- tests/testcases/messages/test-controller.php | 74 +++++++- 2 files changed, 188 insertions(+), 46 deletions(-) diff --git a/includes/bp-messages/classes/class-bp-rest-messages-endpoint.php b/includes/bp-messages/classes/class-bp-rest-messages-endpoint.php index 19510a42..56982e0d 100644 --- a/includes/bp-messages/classes/class-bp-rest-messages-endpoint.php +++ b/includes/bp-messages/classes/class-bp-rest-messages-endpoint.php @@ -197,17 +197,10 @@ public function get_items_permissions_check( $request ) { ); if ( is_user_logged_in() ) { - $user = bp_rest_get_user( $request->get_param( 'user_id' ) ); - - if ( ! $user instanceof WP_User ) { - $retval = new WP_Error( - 'bp_rest_invalid_id', - __( 'Invalid member ID.', 'buddypress' ), - array( - 'status' => 404, - ) - ); - } elseif ( (int) bp_loggedin_user_id() === $user->ID || bp_current_user_can( 'bp_moderate' ) ) { + $user_id = $this->validate_requested_user_id( $request ); + if ( is_wp_error( $user_id ) ) { + $retval = $user_id; + } elseif ( (int) bp_loggedin_user_id() === $user_id || bp_current_user_can( 'bp_moderate' ) ) { $retval = true; } else { $retval = new WP_Error( @@ -277,35 +270,58 @@ public function get_item( $request ) { * @return true|WP_Error */ public function get_item_permissions_check( $request ) { - $error = new WP_Error( + $retval = new WP_Error( 'bp_rest_authorization_required', - __( 'Sorry, you are not allowed to see this thread.', 'buddypress' ), + __( 'Sorry, you are not allowed to perform this action.', 'buddypress' ), array( - 'status' => rest_authorization_required_code(), + 'status' => rest_authorization_required_code() ) ); - $retval = $error; - $user_id = bp_loggedin_user_id(); - if ( ! empty( $request->get_param( 'user_id' ) ) ) { - $user_id = $request->get_param( 'user_id' ); - } - + // Must be logged in. if ( is_user_logged_in() ) { - $thread = $this->get_thread_object( $request->get_param( 'id' ), $user_id ); - - if ( empty( $thread ) ) { - $retval = new WP_Error( - 'bp_rest_invalid_id', - __( 'Sorry, this thread does not exist.', 'buddypress' ), - array( - 'status' => 404, - ) - ); - } elseif ( bp_current_user_can( 'bp_moderate' ) || messages_check_thread_access( $thread->thread_id, $user_id ) ) { - $retval = true; - } else { - $retval = $error; + $thread_id = $request->get_param( 'id' ); + + // Thread ID must be requested. + if ( ! empty( $thread_id ) ) { + + // Get validity of thread. + $thread_valid = messages_is_valid_thread( $thread_id ); + + // Thread not valid. + if ( empty( $thread_valid ) ) { + $retval = new WP_Error( + 'bp_rest_invalid_id', + __( 'Sorry, this thread does not exist.', 'buddypress' ), + array( + 'status' => 404, + ) + ); + + // Thread is valid. + } else { + + // User ID. + $user_id = $this->validate_requested_user_id( $request ); + + // Thread participant. + if ( ! is_wp_error( $user_id ) ) { + $participant = (bool) messages_check_thread_access( $thread_id, $user_id ); + } + + // Invalid user. + if ( is_wp_error( $user_id ) ) { + $retval = $user_id; + + // Moderators can access threads with valid thread participant. + } elseif ( bp_current_user_can( 'bp_moderate' ) && $participant ) { + $retval = true; + + // Valid user must be thread participant. + } elseif ( $participant ) { + $retval = true; + } + } } } @@ -440,10 +456,10 @@ public function update_item( $request ) { // Setting context. $request->set_param( 'context', 'edit' ); - // Updated user id. - $updated_user_id = bp_loggedin_user_id(); - if ( ! empty( $request->get_param( 'user_id' ) ) ) { - $updated_user_id = $request->get_param( 'user_id' ); + // User ID. + $updated_user_id = $this->validate_requested_user_id( $request ); + if ( is_wp_error( $updated_user_id ) ) { + return $updated_user_id; } // Get the thread. @@ -476,6 +492,18 @@ public function update_item( $request ) { } $updated_message = wp_list_filter( $thread->messages, array( 'id' => $message_id ) ); + + // Invalid message ID. + if ( empty( $updated_message ) ) { + return new WP_Error( + 'bp_rest_invalid_id', + __( 'Sorry, this message does not exist.', 'buddypress' ), + array( + 'status' => 404, + ) + ); + } + $updated_message = reset( $updated_message ); /** @@ -680,9 +708,10 @@ public function delete_item( $request ) { // Setting context. $request->set_param( 'context', 'edit' ); - $user_id = bp_loggedin_user_id(); - if ( ! empty( $request->get_param( 'user_id' ) ) ) { - $user_id = $request->get_param( 'user_id' ); + // User ID. + $user_id = $this->validate_requested_user_id( $request ); + if ( is_wp_error( $user_id ) ) { + return $user_id; } // Get the thread before it's deleted. @@ -1039,13 +1068,14 @@ public function get_thread_object( $thread_id, $user_id = 0 ) { $args = array( 'user_id' => $user_id ); } - $thread_object = new BP_Messages_Thread( (int) $thread_id, 'ASC', $args ); + // Validate the thread ID. + $thread_id = messages_is_valid_thread( $thread_id ); - if ( false === (bool) $thread_object::is_valid( $thread_id ) ) { + if ( false === (bool) $thread_id ) { return ''; } - return $thread_object; + return new BP_Messages_Thread( (int) $thread_id, 'ASC', $args ); } /** @@ -1431,4 +1461,46 @@ public function get_collection_params() { */ return apply_filters( 'bp_rest_messages_collection_params', $params ); } + + /** + * Validate the requested user ID. + * + * Falls back to the logged in user ID if the requested user ID is empty or + * if the current user doesn't have moderation capabilities. + * + * Returns a WP_Error if the requested user ID is invalid. + * + * @since 12.7.0 (Backported from 14.5.0) + * + * @param WP_REST_Request $request Full details about the request. + * @return WP_Error|int + */ + private function validate_requested_user_id( $request ) { + + // Get the user ID from the request. + $retval = $request->get_param( 'user_id' ); + + // Maybe fallback/override user ID to logged in ID. + if ( empty( $retval ) || ! bp_current_user_can( 'bp_moderate' ) ) { + $retval = bp_loggedin_user_id(); + $request->set_param( 'user_id', $retval ); + } + + // Get the user object. + $user = bp_rest_get_user( $retval ); + + // Requested user not valid. + if ( ! $user instanceof WP_User ) { + $retval = new WP_Error( + 'bp_rest_invalid_id', + __( 'Invalid member ID.', 'buddypress' ), + array( + 'status' => 404, + ) + ); + } + + // Return ID or error. + return $retval; + } } diff --git a/tests/testcases/messages/test-controller.php b/tests/testcases/messages/test-controller.php index ac29af4c..27762f2a 100644 --- a/tests/testcases/messages/test-controller.php +++ b/tests/testcases/messages/test-controller.php @@ -254,6 +254,30 @@ public function test_get_item_user_is_not_logged_in() { $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() ); } + /** + * @group get_item + */ + public function test_get_item_prevent_counterfeit_user_id() { + $u1 = static::factory()->user->create(); + $u2 = static::factory()->user->create(); + $u3 = static::factory()->user->create(); + $m = $this->bp::factory()->message->create_and_get( array( + 'sender_id' => $u1, + 'recipients' => array( $u2 ), + 'subject' => 'Foo', + ) ); + + $this->bp::set_current_user( $u3 ); + + $request = new WP_REST_Request( 'GET', $this->endpoint_url . '/' . $m->thread_id ); + $request->set_param( 'context', 'view' ); + $request->set_param( 'user_id', $u2 ); + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() ); + $this->assertSame( 403, $response->get_status() ); + } + /** * @group get_item */ @@ -567,19 +591,42 @@ public function test_delete_item_with_user_with_no_access() { ); } + /** + * @group update_item + */ + public function test_update_item_prevent_counterfeit_user_id() { + $u1 = static::factory()->user->create(); + $u2 = static::factory()->user->create(); + $u3 = static::factory()->user->create(); + $m = $this->bp::factory()->message->create_and_get( array( + 'sender_id' => $u1, + 'recipients' => array( $u2 ), + 'subject' => 'Foo', + ) ); + + $this->bp::set_current_user( $u3 ); + + $request = new WP_REST_Request( 'PUT', sprintf( $this->endpoint_url . '/%d', $m->thread_id ) ); + $request->set_param( 'user_id', $u2 ); + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() ); + $this->assertSame( 403, $response->get_status() ); + } + /** * @group delete_item */ public function test_delete_item_user_is_not_logged_in() { $u1 = $this->factory->user->create(); $u2 = $this->factory->user->create(); - $m = $this->bp_factory->message->create( array( + $m = $this->bp::factory()->message->create_and_get( array( 'sender_id' => $u1, 'recipients' => array( $u2 ), 'subject' => 'Foo', ) ); - $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m ); + $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m->thread_id ); $this->assertErrorResponse( 'bp_rest_authorization_required', @@ -588,6 +635,29 @@ public function test_delete_item_user_is_not_logged_in() { ); } + /** + * @group delete_item + */ + public function test_delete_item_prevent_counterfeit_user_id() { + $u1 = static::factory()->user->create(); + $u2 = static::factory()->user->create(); + $u3 = static::factory()->user->create(); + $m = $this->bp::factory()->message->create_and_get( array( + 'sender_id' => $u1, + 'recipients' => array( $u2 ), + 'subject' => 'Foo', + ) ); + + $this->bp::set_current_user( $u3 ); + + $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m->thread_id ); + $request->set_param( 'user_id', $u2 ); + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() ); + $this->assertSame( 403, $response->get_status() ); + } + /** * @group starred */