Skip to content

Improve security of messages endpoint (0.8 branch). #521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dcavins merged 1 commit into from
Jul 7, 2026
Merged

Improve security of messages endpoint (0.8 branch). #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 116 additions & 44 deletions includes/bp-messages/classes/class-bp-rest-messages-endpoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
}
}
}
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
}

/**
Expand Down Expand Up @@ -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;
}
}
74 changes: 72 additions & 2 deletions tests/testcases/messages/test-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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',
Expand All @@ -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
*/
Expand Down
Loading