Dougal Campbell's geek ramblings

The Sucuri Blog has a good dissection of the recent critical WordPress REST API vulnerability. I won’t rehash the details here, but I did want to point out that this is why developers should remember to follow these two rules of defensive programming:

1. Sanitize inputs as early as possible
2. Sanitize outputs as late as possible

In this case, there was a failure to follow the first rule. There are a couple of different places where this could have been handled better.

For one, unless there is a good reason to allow GET or POST variables to override a URL placeholder parameter, then that should be handled appropriately. If there is a reason for it, then those values should be forced to comply with the regular expression or data type that is expected.

The other failure, beyond simple sanitizing, is that the logic of the update_item_permission_check() method simply didn’t properly account for the failure to retrieve a valid result from get_post(). This function will return post data either as an object or an array (depending on parameters), or null upon failure. The logic of the first conditional was looking for a valid post and invalid permissions on the post as an error condition:

if ( $post && ! $this->check_update_permission( $post ) ) {
    return new WP_Error( 'rest_cannot_edit', 
        __( 'Sorry, you are not allowed to edit this post.' ), 
        array( 'status' => rest_authorization_required_code() ) );
}

Instead, it should have reversed the valid post logic, and looked for not a valid post, or invalid permissions:

if ( ! $post || ! $this->check_update_permission( $post ) ) {
    return new WP_Error( 'rest_cannot_edit', 
        __( 'Sorry, you are not allowed to edit this post.' ), 
        array( 'status' => rest_authorization_required_code() ) );
}

And for good measure, it should probably check whether the return value was an instance of WP_Error, for future-proofing. This small change would have detected an invalid post ID, and still performed a valid permissions check for existing posts.

Okay boys and girls, let’s say it all together now… “Security is hard.”