Skip to:
Content

BuddyPress.org

Changeset 14213


Ignore:
Timestamp:
07/07/2026 08:02:20 PM (6 weeks ago)
Author:
dcavins
Message:

Improve security of messages endpoint.

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, johnjamesjacoby, espellcaste, 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).

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/bp-messages/classes/class-bp-messages-rest-controller.php

    r14069 r14213  
    201201
    202202                if ( is_user_logged_in() ) {
    203                         $user = bp_rest_get_user( $request->get_param( 'user_id' ) );
    204 
    205                         if ( ! $user instanceof WP_User ) {
    206                                 $retval = new WP_Error(
    207                                         'bp_rest_invalid_id',
    208                                         __( 'Invalid member ID.', 'buddypress' ),
    209                                         array(
    210                                                 'status' => 404,
    211                                         )
    212                                 );
    213                         } elseif ( (int) bp_loggedin_user_id() === $user->ID || bp_current_user_can( 'bp_moderate' ) ) {
     203                        $user_id = $this->validate_requested_user_id( $request );
     204                        if ( is_wp_error( $user_id ) ) {
     205                                $retval = $user_id;
     206                        } elseif ( (int) bp_loggedin_user_id() === $user_id || bp_current_user_can( 'bp_moderate' ) ) {
    214207                                $retval = true;
    215208                        } else {
    … …  
    297290         */
    298291        public function get_item_permissions_check( $request ) {
    299                 $error = new WP_Error(
     292                $retval = new WP_Error(
    300293                        'bp_rest_authorization_required',
    301                         __( 'Sorry, you are not allowed to see this thread.', 'buddypress' ),
     294                        __( 'Sorry, you are not allowed to perform this action.', 'buddypress' ),
    302295                        array(
    303                                 'status' => rest_authorization_required_code(),
     296                                'status' => rest_authorization_required_code()
    304297                        )
    305298                );
    306299
    307                 $retval  = $error;
    308                 $user_id = bp_loggedin_user_id();
    309                 if ( ! empty( $request->get_param( 'user_id' ) ) ) {
    310                         $user_id = $request->get_param( 'user_id' );
    311                 }
    312 
    313                 $id = $request->get_param( 'id' );
    314 
     300                // Must be logged in.
    315301                if ( is_user_logged_in() ) {
    316                         $thread = BP_Messages_Thread::is_valid( $id );
    317 
    318                         if ( empty( $thread ) ) {
    319                                 $retval = new WP_Error(
    320                                         'bp_rest_invalid_id',
    321                                         __( 'Sorry, this thread does not exist.', 'buddypress' ),
    322                                         array(
    323                                                 'status' => 404,
    324                                         )
    325                                 );
    326                         } elseif ( bp_current_user_can( 'bp_moderate' ) || messages_check_thread_access( $id, $user_id ) ) {
    327                                 $retval = true;
     302                        $thread_id = $request->get_param( 'id' );
     303
     304                        // Thread ID must be requested.
     305                        if ( ! empty( $thread_id ) ) {
     306
     307                                // Get validity of thread.
     308                                $thread_valid = messages_is_valid_thread( $thread_id );
     309
     310                                // Thread not valid.
     311                                if ( empty( $thread_valid ) ) {
     312                                        $retval = new WP_Error(
     313                                                'bp_rest_invalid_id',
     314                                                __( 'Sorry, this thread does not exist.', 'buddypress' ),
     315                                                array(
     316                                                        'status' => 404,
     317                                                )
     318                                        );
     319
     320                                // Thread is valid.
     321                                } else {
     322
     323                                        // User ID.
     324                                        $user_id = $this->validate_requested_user_id( $request );
     325
     326                                        // Thread participant.
     327                                        if ( ! is_wp_error( $user_id ) ) {
     328                                                $participant = (bool) messages_check_thread_access( $thread_id, $user_id );
     329                                        }
     330
     331                                        // Invalid user.
     332                                        if ( is_wp_error( $user_id ) ) {
     333                                                $retval = $user_id;
     334
     335                                        // Moderators can access threads with valid thread participant.
     336                                        } elseif ( bp_current_user_can( 'bp_moderate' ) && $participant ) {
     337                                                $retval = true;
     338
     339                                        // Valid user must be thread participant.
     340                                        } elseif ( $participant ) {
     341                                                $retval = true;
     342                                        }
     343                                }
    328344                        }
    329345                }
    … …  
    452468        public function update_item( $request ) {
    453469
    454                 // Updated user id.
    455                 $updated_user_id = bp_loggedin_user_id();
    456                 if ( ! empty( $request->get_param( 'user_id' ) ) ) {
    457                         $updated_user_id = $request->get_param( 'user_id' );
     470                // User ID.
     471                $updated_user_id = $this->validate_requested_user_id( $request );
     472                if ( is_wp_error( $updated_user_id ) ) {
     473                        return $updated_user_id;
    458474                }
    459475
    … …  
    488504
    489505                $updated_message = wp_list_filter( $thread->messages, array( 'id' => $message_id ) );
     506
     507                // Invalid message ID.
     508                if ( empty( $updated_message ) ) {
     509                        return new WP_Error(
     510                                'bp_rest_invalid_id',
     511                                __( 'Sorry, this message does not exist.', 'buddypress' ),
     512                                array(
     513                                        'status' => 404,
     514                                )
     515                        );
     516                }
     517
    490518                $updated_message = reset( $updated_message );
    491519
    … …  
    689717         */
    690718        public function delete_item( $request ) {
    691                 $user_id = bp_loggedin_user_id();
    692                 if ( ! empty( $request->get_param( 'user_id' ) ) ) {
    693                         $user_id = $request->get_param( 'user_id' );
     719                // User ID.
     720                $user_id = $this->validate_requested_user_id( $request );
     721                if ( is_wp_error( $user_id ) ) {
     722                        return $user_id;
    694723                }
    695724
    … …  
    10891118
    10901119                // Validate the thread ID.
    1091                 $thread_id = BP_Messages_Thread::is_valid( $thread_id );
     1120                $thread_id = messages_is_valid_thread( $thread_id );
    10921121
    10931122                if ( false === (bool) $thread_id ) {
    … …  
    16201649                return apply_filters( 'bp_rest_messages_collection_params', $params );
    16211650        }
     1651
     1652        /**
     1653         * Validate the requested user ID.
     1654         *
     1655         * Falls back to the logged in user ID if the requested user ID is empty or
     1656         * if the current user doesn't have moderation capabilities.
     1657         *
     1658         * Returns a WP_Error if the requested user ID is invalid.
     1659         *
     1660         * @since 15.0.0
     1661         *
     1662         * @param WP_REST_Request $request Full details about the request.
     1663         * @return WP_Error|int
     1664         */
     1665        private function validate_requested_user_id( $request ) {
     1666
     1667                // Get the user ID from the request.
     1668                $retval = $request->get_param( 'user_id' );
     1669
     1670                // Maybe fallback/override user ID to logged in ID.
     1671                if ( empty( $retval ) || ! bp_current_user_can( 'bp_moderate' ) ) {
     1672                        $retval = bp_loggedin_user_id();
     1673                        $request->set_param( 'user_id', $retval );
     1674                }
     1675
     1676                // Get the user object.
     1677                $user = bp_rest_get_user( $retval );
     1678
     1679                // Requested user not valid.
     1680                if ( ! $user instanceof WP_User ) {
     1681                        $retval = new WP_Error(
     1682                                'bp_rest_invalid_id',
     1683                                __( 'Invalid member ID.', 'buddypress' ),
     1684                                array(
     1685                                        'status' => 404,
     1686                                )
     1687                        );
     1688                }
     1689
     1690                // Return ID or error.
     1691                return $retval;
     1692        }
    16221693}

  • trunk/tests/phpunit/testcases/messages/test-controller.php

    r14070 r14213  
    333333         * @group get_item
    334334         */
     335        public function test_get_item_prevent_counterfeit_user_id() {
     336                $u1 = static::factory()->user->create();
     337                $u2 = static::factory()->user->create();
     338                $u3 = static::factory()->user->create();
     339                $m  = $this->bp::factory()->message->create_and_get( array(
     340                        'sender_id'  => $u1,
     341                        'recipients' => array( $u2 ),
     342                        'subject'    => 'Foo',
     343                ) );
     344
     345                $this->bp::set_current_user( $u3 );
     346
     347                $request = new WP_REST_Request( 'GET', $this->endpoint_url . '/' . $m->thread_id );
     348                $request->set_param( 'context', 'view' );
     349                $request->set_param( 'user_id', $u2 );
     350                $response = $this->server->dispatch( $request );
     351
     352                $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() );
     353                $this->assertSame( 403, $response->get_status() );
     354        }
     355
     356        /**
     357         * @group get_item
     358         */
    335359        public function test_get_item_with_edit_context() {
    336360                $u1 = static::factory()->user->create();
    … …  
    810834
    811835        /**
     836         * @group update_item
     837         */
     838        public function test_update_item_prevent_counterfeit_user_id() {
     839                $u1 = static::factory()->user->create();
     840                $u2 = static::factory()->user->create();
     841                $u3 = static::factory()->user->create();
     842                $m  = $this->bp::factory()->message->create_and_get( array(
     843                        'sender_id'  => $u1,
     844                        'recipients' => array( $u2 ),
     845                        'subject'    => 'Foo',
     846                ) );
     847
     848                $this->bp::set_current_user( $u3 );
     849
     850                $request = new WP_REST_Request( 'PUT', sprintf( $this->endpoint_url . '/%d', $m->thread_id ) );
     851                $request->set_param( 'user_id', $u2 );
     852                $response = $this->server->dispatch( $request );
     853
     854                $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() );
     855                $this->assertSame( 403, $response->get_status() );
     856        }
     857
     858        /**
    812859         * @group delete_item
    813860         */
    … …  
    903950                $u1 = static::factory()->user->create();
    904951                $u2 = static::factory()->user->create();
    905                 $m  = $this->bp::factory()->message->create(
    906                         array(
    907                                 'sender_id'  => $u1,
    908                                 'recipients' => array( $u2 ),
    909                                 'subject'    => 'Foo',
    910                         )
    911                 );
    912 
    913                 $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m );
     952                $m  = $this->bp::factory()->message->create_and_get(
     953                        array(
     954                                'sender_id'  => $u1,
     955                                'recipients' => array( $u2 ),
     956                                'subject'    => 'Foo',
     957                        )
     958                );
     959
     960                $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m->thread_id );
    914961                $request->set_param( 'context', 'edit' );
    915962
    … …  
    919966                        rest_authorization_required_code()
    920967                );
     968        }
     969
     970        /**
     971         * @group delete_item
     972         */
     973        public function test_delete_item_prevent_counterfeit_user_id() {
     974                $u1 = static::factory()->user->create();
     975                $u2 = static::factory()->user->create();
     976                $u3 = static::factory()->user->create();
     977                $m  = $this->bp::factory()->message->create_and_get( array(
     978                        'sender_id'  => $u1,
     979                        'recipients' => array( $u2 ),
     980                        'subject'    => 'Foo',
     981                ) );
     982
     983                $this->bp::set_current_user( $u3 );
     984
     985                $request = new WP_REST_Request( 'DELETE', $this->endpoint_url . '/' . $m->thread_id );
     986                $request->set_param( 'user_id', $u2 );
     987                $response = $this->server->dispatch( $request );
     988
     989                $this->assertErrorResponse( 'bp_rest_authorization_required', $response, rest_authorization_required_code() );
     990                $this->assertSame( 403, $response->get_status() );
    921991        }
    922992
Note: See TracChangeset for help on using the changeset viewer.