WP oEmbed: Validate the "Secret" When Used in `document.querySelectorAll()`

[mdawaffe]

In the data sent to us from the embedded iframe by postMessage(), the secret value is being used directly in a document.querySelectorAll() call without first being validated or escaped.

In theory, this could lead to some broken embeds.

Suggested hardening patch attached: There's no reason to try and escape this data correctly. Let's just reject if the secret does not conform to the format we expect.

[TobiasBg]

How about just prepending

if ( /[^a-zA-Z0-9]/.test( data.secret ) ) { 
    return; 
}

if we return anyways? Would make for a simpler logic, IMO.

[mdawaffe]

TobiasBg's patch

[mdawaffe]

Works for me. Updated patch: attachment:34831.2.diff​

[swissspidy]

+1 to this enhancement and the second patch.

[wonderboymusic]

Looks good.

[pento]

Is /[^a-zA-Z0-9]/.test( data.secret ) or data.secret.match( /[^a-zA-Z0-9]/ ) better?

[swissspidy]

Replying to pento:

Is /[^a-zA-Z0-9]/.test( data.secret ) or data.secret.match( /[^a-zA-Z0-9]/ ) better?

test() returns a bool, whereas match() returns all the matches. For that reason, test() is much faster.

MDN ​recommends using search() if you need to know if a string matches a regular expression. search() returns the index of the first match or -1.

​This comparison between all those functions shows that search() and test() are both much, much faster than the other methods.

[pento]

Well, I'm convinced. Let's commit it.

[mdawaffe]

Compare window objects

[mdawaffe]

Another piece of hardening: We can't do the normal postMessage() origin checks (sandboxed iframes have sandboxed origins), but we can ensure that the message event's source (a window object) is the same as the iframe's window.

This protects against some potential, weird information disclosure bug with the secret. That is, with this extra check, the secret does not need to be private; it just becomes a unique ID.

Combined patch attached: attachment:34831.3.diff​.

[swissspidy]

I've tested the latest patch, even by embedding from different domains as well. Works as advertised!

[wonderboymusic]

In 35761:

WP oEmbed: validate the secret send via postMessage in wp.receiveEmbedMessage. Also, compare window instances.

In the data sent to us from the embedded iframe by postMessage(), the secret value is being used directly in a document.querySelectorAll() call without first being validated or escaped.

In theory, this could lead to some broken embeds.

Props mdawaffe.
Fixes #34831.