Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#45477 closed enhancement (wontfix)

Disable REST API reflection of request Origin header in response Access-Control-Allow-Origin

Reported by: bjornw's profile BjornW Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

At this moment the WordPress REST API automatically uses the Origin: $url header from a request in it's Access-Control-Allow-Origin: $url response header. See wp-includes/rest-api.php

The effect of this in practice, is similar to that of setting the header to "*" which according to MDN means:

"For requests without credentials, the literal value "*" can be specified, as a wildcard; the value tells browsers to allow requesting code from any origin to access the resource."

Thus any random origin (external URL) may access the (resources of) the REST API.

WordPress combines this with the Access-Control-Allow-Credentials: true header which according to W3C might be a potential security and/or privacy risk if used with "*". I presume it might also be a potential issue using this without any validation or verification of the incoming URL before sending out crendentials (cookies or other).

Considering the above, in my opinion, the current CORS headers implementation in WordPress' REST API diminishes CORS' safety measures needlessly.

If WordPress wants the REST API to be more secure I'd suggest to limit access to the REST API using a 'safe-list' of of Origin URLs which are allowed access to the REST API.

Any incoming request with an Origin header NOT on the list will NOT receive the required Access-Control-* headers. If the Origin is on the 'safe-list' the Origin will be set in the Access-Control-Allow-Origin header.

Luckily WordPress already has an API for this!

Using the Allow Origin API we can add Origin URL's to a 'safe-list' and only grant access to those on the 'safe-list'.

I've used the Allow Origin API to create a plugin which adds an Origin to the 'safe-list' and replaces the default rest_send_cors_headers() with the one in my plugin. It is almost identical except for a check making sure the Origin URL is on the 'safe-list' before sending out Access-Control-* headers, otherwise none are sent.

Attachments (5)

45477.diff (2.1 KB) - added by bjornw 6 years ago.
45477.2.diff (955 bytes) - added by BjornW 6 years ago.
Using Allowed Origin API to verify an Origin before sending CORS headers
screenshot-without-origin-verification.png (74.0 KB) - added by BjornW 6 years ago.
The Request Origin header is used without any verification before sending out the Response CORS headers
screenshot-origin-is-verified-and-not-on-the-safe-list.png (45.6 KB) - added by BjornW 6 years ago.
The Request Origin header is verified and NOT found on the safe-list. CORS headers are not sent.
screenshot-origin-is-verified-and-found-safe.png (60.6 KB) - added by BjornW 6 years ago.
The Request Origin header is verified and part of the safe-list. CORS headers are sent out.

Download all attachments as: .zip

Change History (15)

@bjornw
6 years ago

@BjornW
6 years ago

Using Allowed Origin API to verify an Origin before sending CORS headers

#1 @BjornW
6 years ago

  • Keywords has-patch 2nd-opinion added

This patch uses the Allow Origin API to check if an incoming Origin URL is part of the allowed Origins safe-list. By default, the only Origin URLs allowed are the current host (both http and https).

Adding an Origin to the safe-list can be done using the 'allowed_http_origins' filter.

@BjornW
6 years ago

The Request Origin header is used without any verification before sending out the Response CORS headers

@BjornW
6 years ago

The Request Origin header is verified and NOT found on the safe-list. CORS headers are not sent.

@BjornW
6 years ago

The Request Origin header is verified and part of the safe-list. CORS headers are sent out.

#2 @BjornW
6 years ago

The last two screenshots show the behaviour after applying my patch. The Origin header is verified and only if it is found on the safe-list of allowed http origins the CORS headers are sent. Otherwise the CORS headers found in rest_send_cors_headers() are not sent.

#3 in reply to: ↑ description @BjornW
6 years ago

Replying to BjornW:

Here's a link to the repo of the plugin (WP-No-Auto-Origin-Header) I was referring to in my initial post. It might be useful until this behavior is solved in WordPress Core.

#4 @swissspidy
6 years ago

Related tickets: #40011, #38060

#5 @swissspidy
6 years ago

  • Type changed from defect (bug) to enhancement

Marking as enhancement since this is intentional behavior. Citing [40600]:

Browsers send an "Origin: null" header value for file and data URLs, as they can be generated by any document, and their origin is not guaranteed. Since we want to allow any URL to access the API (intentionally disabling the CORS protections), we need to special-case the non-URL "null" value.

#6 @BjornW
6 years ago

@swissspidy and I had a quick chat about this.

@swissspidy: According to older HackerOne reports WordPress should not be vulnerable to exploitation of this due to WordPress requiring a nonce to be sent with each request.

@bjornw: However I worry about plugins adding end-points and making mistakes. My patch makes sure only Allowed Origins are sent the proper headers. Which seems like a good idea to me.
I'm not a CORS expert and it is rather complex, and I also understand the need for a user-friendly default for WordPress, but I'd suggest we at least reconsider the current implementation and see if it is still the best option. Especially since the REST API cannot be easily switched off anymore with Gutenberg out.

Other projects had a similar implementation (I don't know if they used nonces) and considered it an issue:

Why should we allow any Origin to get the CORS headers without verification? Is there something I've overlooked?

This ticket was mentioned in Slack in #core-restapi by bjornw. View the logs.


6 years ago

#8 in reply to: ↑ 7 @BjornW
6 years ago

Replying to slackbot:

This ticket was mentioned in Slack in #core-restapi by bjornw. View the logs.

Here's a short summary (for those not having access to Slack):

The current behaviour of reflecting the incoming Origin as-is, is an intentional design decision (as mentioned before and according to @rmccue:

"tl;dr: CORS is built for CSRF protection, but WordPress already has a system for that (nonces), so we "disable" CORS as it gets in the way of alternative authentication schemes"

I don't understand why verification of an Origin would stand in the way of alternative authentication schemes?

The current view of WordPress on the REST API is according to @rmccue:

"it's a design decision to expose data from the REST API to all origins; you should be able to override in plugins easily"

And my plugin (I'm sure there are more) does this.

Personally I'd expect WordPress to verify Origins before sending CORS headers by default. Instead it's intentionally open to any Origin by default. I disagree with this, but I agree to disagree.

PS: As far as I know this behavior was not documented anywhere in the REST API handbook or FAQ. I've opened up a pull-request to remedy this by adding it to the FAQ. Hopefully this saves people some time.

This ticket was mentioned in Slack in #core-restapi by desrosj. View the logs.


6 years ago

#10 @TimothyBlynJacobs
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Closing based on @rmccue's feedback and the docs PR adding an FAQ has been merged.

Note: See TracTickets for help on using tickets.