Skip to content

Commit

Permalink
REST API: Only check password value in query parameters while checkin…
Browse files Browse the repository at this point in the history
…g post permissions.

The `password` property which gets sent as part of a request POST body while setting a post's password should not be checked when calculating post visibility permissions.

That value in the request body is intended to update the post, not to authenticate, and may be malformed or an invalid non-string type which would cause a fatal when checking against the hashed post password value.

Query parameter `?password=` values are the correct interface to check, and are also guaranteed to be strings.

Props mlf20, devansh016, antonvlasenko, TimothyBlynJacobs, kadamwhite.
Fixes #61837.



git-svn-id: https://develop.svn.wordpress.org/trunk@59036 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
kadamwhite committed Sep 17, 2024
1 parent d3d02c4 commit 15b7d2a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,9 @@ public function get_item_permissions_check( $request ) {
);
}

if ( $post && ! empty( $request['password'] ) ) {
if ( $post && ! empty( $request->get_query_params()['password'] ) ) {
// Check post password, and return error if invalid.
if ( ! hash_equals( $post->post_password, $request['password'] ) ) {
if ( ! hash_equals( $post->post_password, $request->get_query_params()['password'] ) ) {
return new WP_Error(
'rest_post_incorrect_password',
__( 'Incorrect post password.' ),
Expand Down
45 changes: 45 additions & 0 deletions tests/phpunit/tests/rest-api/rest-posts-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -2232,6 +2232,51 @@ public function test_get_post_with_password_without_permission() {
$this->assertTrue( $data['excerpt']['protected'] );
}

/**
* @ticket 61837
*/
public function test_get_item_permissions_check_while_updating_password() {
$endpoint = new WP_REST_Posts_Controller( 'post' );

$request = new WP_REST_Request( 'POST', sprintf( '/wp/v2/posts/%d', self::$post_id ) );
$request->set_url_params( array( 'id' => self::$post_id ) );
$request->set_body_params(
$this->set_post_data(
array(
'id' => self::$post_id,
'password' => '123',
)
)
);
$permission = $endpoint->get_item_permissions_check( $request );

// Password provided in POST data, should not be used as authentication.
$this->assertNotWPError( $permission, 'Password in post body should be ignored by permissions check.' );
$this->assertTrue( $permission );
}

/**
* @ticket 61837
*/
public function test_get_item_permissions_check_while_updating_password_with_invalid_type() {
$endpoint = new WP_REST_Posts_Controller( 'post' );

$request = new WP_REST_Request( 'POST', sprintf( '/wp/v2/posts/%d', self::$post_id ) );
$request->set_url_params( array( 'id' => self::$post_id ) );
$request->set_body_params(
$this->set_post_data(
array(
'id' => self::$post_id,
'password' => 123,
)
)
);
$permission = $endpoint->get_item_permissions_check( $request );

$this->assertNotWPError( $permission, 'Password in post body should be ignored by permissions check even when it is an invalid type.' );
$this->assertTrue( $permission );
}

/**
* The post response should not have `block_version` when in view context.
*
Expand Down

0 comments on commit 15b7d2a

Please sign in to comment.