PHP Warnings on invalid requests to wp-json/oembed/1.0/embed

[dd32]

The url parameter can cause PHP Warnings on invalid requests. Such requests shouldn't be made, but Vulnerability scanners have a tendancy to fuzz the API since it's linked to from most pages/posts.

​https://...../wp-json/oembed/1.0/embed?url[]=example

PHP Warning: ltrim() expects parameter 1 to be string, array given in wp-includes/formatting.php on line 4297

This happens as the url fields sanitize_callback is set to esc_url_raw which doesn't handle invalid inputs well.
Adding a validate_callback to check it's a string works properly.

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

[dd32]

PR #236

For some reason adding is_string as the validate_callback didn't help. (Which is why it's an anonymous function wrapping it)

It looks like the type => string parameter still causes a PHP Notice as it's simply run through strval() and not actually validated/rejected.

[TimothyBlynJacobs]

Hm, the type should be sufficient. rest_validate_value_from_schema does an is_string check. That whole check should be identical to:

{
  "type": "string",
  "format": "uri"
}

[TimothyBlynJacobs]

Yeah, when I apply that change, and I make the example request, I get this error and no PHP warnings.

{
  "code": "rest_invalid_param",
  "message": "Invalid parameter(s): url",
  "data": {
    "status": 400,
    "params": {
      "url": "url is not of type string."
    }
  }
}

[dd32]

@TimothyBlynJacobs Thanks for following up, Are there any build steps that need to be done?

It doesn't look like rest_validate_request_arg() runs at all in my test environments with just type specified.
Using { type: string, validate_callback: rest_validate_request_arg } appears to work though.

For some reason adding is_string as the validate_callback didn't help. (Which is why it's an anonymous function wrapping it)

For reference, it probably didn't work for me as is_string() needs 1 param and using it here as it was, passed 3.

It looks like the type => string parameter still causes a PHP Notice as it's simply run through strval() and not actually validated/rejected.

I can't duplicate the PHP Notice now, I'm not sure what combination of things I set to trigger that.

[TimothyBlynJacobs]

Ah, you'd need to drop the sanitize_callback as well.

Routes that extend WP_REST_Controller and use get_endpoint_args_for_item_schema automatically apply rest_(sanitize|validate)_request_arg for the sanitize and validate callbacks.

The oembed controller predates that. Instead, WP_REST_Request::sanitize_params will set rest_parse_request_arg as the sanitization callback if one isn't set, and that will apply rest_sanitize_value_from_schema and rest_validate_value_from_schema.

So the definition should be.

array(
        'description' => __( 'The URL of the resource for which to fetch oEmbed data.' ),
        'type'        => 'string',
        'format'      => 'uri',
        'required'    => true,
)

[dd32]

ahah! that explains it, thanks @TimothyBlynJacobs

I've updated the PR and tested it locally, it seems to do the trick :)

(I'll throw this over to you to commit if you want)

[TimothyBlynJacobs]

In 47755:

REST API: Validate that the oembed url parameter is a string.

This prevents a PHP warning from being issued by esc_url_raw when a non-string value is provided.

Props dd32.
Fixes #49991.

Merged in [r47755](https://core.trac.wordpress.org/changeset/47755), 6ab90a2.