REST API: Uncaught TypeError when post password is provided as integer

[mlf20]

Upon creating a fresh instance of WordPress and setting up the REST API a malformed post request REST API endpoint /wp-json/wp/v2/posts/{id} results in an exception in the endpoint.

Request body:
'{"date": "2024-08-07T02:04:37", "date_gmt": "2024-08-07T02:04:37", "slug": "hello-world", "status": "publish", "password": 99999999, "author": 1, "featured_media": 0, "comment_status": "open", "ping_status": "open", "format": "", "sticky": false, "template": "PE9PCU5W", "categories": [], "tags": [""]}'

Command to reproduce

curl -X POST "[WORDPRESSDOMAIN]/wp-json/wp/v2/posts/1" --data '{"date": "2024-08-07T02:04:37", "date_gmt": "2024-08-07T02:04:37", "slug": "hello-world", "status": "publish", "password": 99999999, "author": 1, "featured_media": 0, "comment_status": "open", "ping_status": "open", "format": "", "sticky": false, "template": "PE9PCU5W", "categories": [], "tags": [""]}' -H 'Authorization: Basic [ACCESS_TOKEN]' -H 'Content-Type: application/json'

Stacktrace

<br />
<b>Fatal error</b>:  Uncaught TypeError: hash_equals(): Argument #2 ($user_string) must be of type string, int given in /var/www/html/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php:496
Stack trace:
#0 /var/www/html/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php(496): hash_equals('', 99999999)
#1 /var/www/html/wp-includes/rest-api.php(821): WP_REST_Posts_Controller-&gt;get_item_permissions_check(Object(WP_REST_Request))
#2 /var/www/html/wp-includes/class-wp-hook.php(308): rest_send_allow_header(Object(WP_REST_Response), Object(WP_REST_Server), Object(WP_REST_Request))
#3 /var/www/html/wp-includes/plugin.php(205): WP_Hook-&gt;apply_filters(Object(WP_REST_Response), Array)
#4 /var/www/html/wp-includes/rest-api/class-wp-rest-server.php(466): apply_filters('rest_post_dispa...', Object(WP_REST_Response), Object(WP_REST_Server), Object(WP_REST_Request))
#5 /var/www/html/wp-includes/rest-api.php(410): WP_REST_Server-&gt;serve_request('/wp/v2/posts/1')
#6 /var/www/html/wp-includes/class-wp-hook.php(308): rest_api_loaded(Object(WP))
#7 /var/www/html/wp-includes/class-wp-hook.php(332): WP_Hook-&gt;apply_filters('', Array)
#8 /var/www/html/wp-includes/plugin.php(565): WP_Hook-&gt;do_action(Array)
#9 /var/www/html/wp-includes/class-wp.php(399): do_action_ref_array('parse_request', Array)
#10 /var/www/html/wp-includes/class-wp.php(780): WP-&gt;parse_request('')
#11 /var/www/html/wp-includes/functions.php(1334): WP-&gt;main('')
#12 /var/www/html/wp-blog-header.php(16): wp()
#13 /var/www/html/index.php(17): require('/var/www/html/w...')
#14 {main}
  thrown in <b>/var/www/html/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php</b> on line <b>496</b><br />
{"code":"internal_server_error","message":"<p>There has been a critical error on this website.<\/p><p><a href=\"https:\/\/wordpress.org\/documentation\/article\/faq-troubleshooting\/\">Learn more about troubleshooting WordPress.<\/a><\/p>

Add type check for post password field it should be a string.
Trac ticket: https://core.trac.wordpress.org/ticket/61837

[antonvlasenko]

Reproduction Report

Environment

• WordPress: 6.7-alpha-58576-src
• PHP: 7.3.33
• Server: Apache/2.4.57 (Unix) PHP/7.3.33
• Database: mysqli (Server: 5.7.43 / Client: mysqlnd 5.0.12-dev)
• Browser: Safari 17.6 (macOS)
• Theme: Twenty Twenty-Four 1.2
• MU-Plugins: None activated
• Plugins:
  • JSON Basic Authentication 0.1

Actual Results

• The bug is reproducible.

Additional Notes

[13-Aug-2024 21:07:21 UTC] PHP Warning:  hash_equals(): Expected user_string to be a string, int given in wordpress.host/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php on line 502
[13-Aug-2024 21:07:21 UTC] PHP Stack trace:
[13-Aug-2024 21:07:21 UTC] PHP   1. {main}() wordpress.host/src/index.php:0
[13-Aug-2024 21:07:21 UTC] PHP   2. require_once() wordpress.host/src/index.php:19
[13-Aug-2024 21:07:21 UTC] PHP   3. require() wordpress.host/src/_index.php:17
[13-Aug-2024 21:07:21 UTC] PHP   4. wp($query_vars = *uninitialized*) wordpress.host/src/wp-blog-header.php:16
...
<the stack trace is too long to post it here>

Supplemental Artifacts

[kadamwhite]

The issue being reported is that an error is thrown when a numeric password is provided as an integer instead of as a string, correct?

The schema for the password property of a Post object specifies it is expected to be provided a string, not an integer: see ​WP_REST_Posts_Controller::get_item_schema. The column for post password itself is a varchar(255), and we use a regular textual input box for the password on the WordPress UI and frontend. So there's an argument that an integer password shouldn't be accepted, because you should supply the password as a string, the way the schema expects.

That said, I agree that a 500 due to a bad type internally is NOT the correct response! Thanks for the report.

The ​linked PR adds a specific type check within the password validation function. Before landing that (or adding a type coercion to ensure the value is a string before passing to hash_equals(), so that we don't error out early without a good message), we should check whether there is a missing layer of existing validation -- is this hash check running before schema validation, or is schema validation not kicking in at all when a non-string is provided?

I'd personally prefer we unify this check with the other schema validation, rather than introducing a one-off check into this function.

[kadamwhite]

OK this one's cool. :D The current patch is not ideal, because we DO validate that value against the schema already, and return an error. But then we call the permission_callback within rest_send_allow_header, and since the password is in the unexpected type, things blow up while trying to send the error response back to the client.

We're specifically running into a situation where we expect to be checking the password query parameter used for GET access, when we're actually processing a POST that updates the password property of a post.

cc @TimothyBlynJacobs -- I'd like to add a basic type coercion within the permission callback to fix the immediate issue, but it might be worth looking at whether the rest_send_allow_header method is doing too much.

[kadamwhite]

Update after in-person discussion with @TimothyBlynJacobs , the core issue we should fix is that the get item check should not be considering post body password, only query parameter password.

Trac ticket: https://core.trac.wordpress.org/ticket/61837

Adding unit tests for failing situation described in ticket, then will update with a merged patch.

[kadamwhite]

In 59036:

REST API: Only check password value in query parameters while checking 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.

Merged in 15b7d2a86885a6c83b520277f97b1debe31048fb