JS: False negative - unsafe postMessage handler not detected

[danilishinyar]

Description of the issue

I tested CodeQL on JS, which handles postMessage validation. The rule for detecting the absence of an origin check in the postMessage handler function does not cover all possible ways to bypass that check. Firstly, the rule doesn't identify cases where only event.source is checked. Such verification can be easily bypassed.

https://book.hacktricks.xyz/pentesting-web/postmessage-vulnerabilities#bypassing-e.source

/** Gets a reference to the origin or the source of a postmessage event. */
DataFlow::SourceNode sourceOrOrigin(PostMessageHandler handler) {
  result = source(DataFlow::TypeTracker::end(), handler) or
  result = origin(DataFlow::TypeTracker::end(), handler)
}

Secondly, checks like "safeOrigin".startsWith(event.origin) and "safeOrigin".endsWith(event.origin) can also be bypassed.

Lastly, there are methods to bypass the equality operation as well, so it's only safe to use strict equality operators (===and !==).

For more details on bypassing origin checks, you can refer to: https://book.hacktricks.xyz/pentesting-web/postmessage-vulnerabilities#origin-check-bypasses, https://book.hacktricks.xyz/pentesting-web/postmessage-vulnerabilities#e.origin-window.origin-bypass

predicate hasOriginCheck(PostMessageHandler handler) {
  // event.origin === "constant"
  exists(EqualityTest test | sourceOrOrigin(handler).flowsToExpr(test.getAnOperand()))
  or
  // set.includes(event.source)
  exists(InclusionTest test | sourceOrOrigin(handler).flowsTo(test.getContainedNode()))
  or
  // "safeOrigin".startsWith(event.origin)
  exists(StringOps::StartsWith starts |
    origin(DataFlow::TypeTracker::end(), handler).flowsTo(starts.getSubstring())
  )
  or
  // "safeOrigin".endsWith(event.origin)
  exists(StringOps::EndsWith ends |
    origin(DataFlow::TypeTracker::end(), handler).flowsTo(ends.getSubstring())
  )
}

The only secure way to handle postMessage is to always check event.origin.

CodeQL Rule Rule: MissingOriginCheck.ql

Thank you for your time!

[erik-krogh]

Firstly, the rule doesn't identify cases where only event.source is checked. Such verification can be easily bypassed.

https://book.hacktricks.xyz/pentesting-web/postmessage-vulnerabilities#bypassing-e.source

I don't agree on that.
Yes, an attacker can easily set e.source to be null, but an attacker cannot set e.source to any other value.
So any validation that e.source is a non-null value cannot be easily bypassed.

Secondly, checks like "safeOrigin".startsWith(event.origin) and "safeOrigin".endsWith(event.origin) can also be bypassed.

Technically yes, but mostly no.
Assume that const safe = "https://example.org".
There is no other domain I can buy that satisfies safe.startsWith(event.origin).
E.g. the string "https://examp" would be a "bypass", but I can't use that for an attack.

Yes, bypasses can exist for other values of safe, but that doesn't mean that a .startsWith(...) check is always unsafe.

Lastly, there are methods to bypass the equality operation as well, so it's only safe to use strict equality operators (===and !==).

The only bypass mentioned in your sources is if both sides are null. So not relevant if you're checking against any non-null value.

The only secure way to handle postMessage is to always check event.origin.

Again, no.
E.g. checking that e.source is a known non-null value is safe.

---

In general our analysis doesn't try to flag every program that could possibly be unsafe, because it's extremely hard to guarantee that no exploit can exist.
So trying to flag everything that could be unsafe would result in an overwhelming amount of false positives.
(Here is a nice paper by Google that describes what happens if you have too many false positives)