Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#40011 closed defect (bug) (fixed)

Do not add scheme prefix to "null" origin in wp-json's Access-Control-Allow-Origin header

Reported by: vicshih's profile vicshih Owned by: rmccue's profile rmccue
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: commit
Focuses: Cc:

Description

In some contexts (e.g. privacy-sensitive), the Origin header is "null".

In these cases the REST API responds with an Access-Control-Allow-Origin header with the value "http://$origin", since the original origin goes through esc_url_raw() before rendering. The browser then does not consider these equivalent and aborts the request with:

The 'Access-Control-Allow-Origin' header has a value 'http://null' that is not equal to the supplied origin. Origin 'null' is therefore not allowed access.

Attachments (1)

40011.diff (660 bytes) - added by joehoyle 7 years ago.

Download all attachments as: .zip

Change History (7)

#2 @joehoyle
7 years ago

  • Milestone changed from Awaiting Review to 4.8
  • Owner set to joehoyle
  • Status changed from new to assigned

Thanks for the report! Yeah we should just not send the header in this case. I can do a patch.

#3 @joehoyle
7 years ago

I've attached a patch to fix this use-case. To provide some extra details, when a browser is run without an origin (such as from file:// or potentially iframes, requests from other sandboxed documents, the browser will send the literally string null as the header in the form of Origin: null. This leads to the case reported here.

Practically speaking, Origin: null in our case should be fine to respond with Access-Control-Allow-Origin: "null", however head the warning from the w3c: https://w3c.github.io/webappsec-cors-for-developers/#avoid-returning-access-control-allow-origin-null

It may seem safe to return Access-Control-Allow-Origin: "null" , but the serialization of the Origin of any resource that uses a non-hierarchical scheme (such as data: or file: ) and sandboxed documents is defined to be "null". Many User Agents will grant such documents access to a response with an Access-Control-Allow-Origin: "null" header, and any origin can create a hostile document with a "null" Origin. The "null" value for the ACAO header should therefore be avoided.

The simple string comparison of CORS as applied to "null" is controversial. Some believe that "null" should be treated as a keyword token indicating the lack of an Origin, which, when tested, should never compare as equal to another "null" Origin. (As is the case with null values in SQL, for example.) It is unwise to build systems which rely on the "null" equals "null" comparison as this behavior may change in the future.

Given we are basically disabling CORS protection (we use a nonce for those purposes) then I think it our case we are ok to return null. Our goal is to allow the browser / browser applications to use the REST API cross-origin, and that should support file:// etc origins.

Feedback from perhaps @rmccue / @jnylen0 would be good.

@joehoyle
7 years ago

#4 @joehoyle
7 years ago

Ping ping @rmccue

#5 @rmccue
7 years ago

  • Keywords commit added
  • Owner changed from joehoyle to rmccue
  • Status changed from assigned to reviewing

Looks good to me.

#6 @rmccue
7 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 40600:

REST API: Allow "Origin: null" from file: URLs.

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.

Props joehoyle.
Fixes #40011.

Note: See TracTickets for help on using tickets.