postMessage origin and source checks in the Hyperswitch web SDK, and why the fix should ship alone

Hyperswitch Web validated neither origin nor source before confirming a payment from a message. The sender checks are solid; the pull request also bundles behavior changes and describes features that are not in the code.

Category
Code review
Published
Reading time
3 min read

The problem

A merchant page loads the Hyperswitch loader, which mounts the payment form in an iframe and opens overlays for 3-D Secure and wallets. The parts talk through window.postMessage. Issue #1823 describes the gap:

The merchant-side handleSdkConfirm message listener does not validate MessageEvent.origin or MessageEvent.source before invoking payment confirmation. A cross-origin frame or window holding a reference to the merchant checkout can send a confirmation command that the loader forwards to its mounted payment iframe.

Because the forwarded message then comes from the legitimate merchant page, it also passed the card component's parent-window check. The issue rates it medium severity.

The proposed fix

A new module pins both the sending window and a concrete origin:

src/Utilities/PaymentMessageSecurity.res
// A window identity alone survives navigation; always authenticate its origin too.
let isConcreteOrigin = origin => origin !== "" && origin !== "*" && origin !== "null"

let isExpectedSender = (~source, ~origin, ~eventSource, ~eventOrigin) =>
  isConcreteOrigin(origin) && eventSource === source && eventOrigin === origin

The confirmation handler now runs only for messages from the payment iframe, and return URLs are compared in full (protocol, host, path, hash and query, except the parameters the SDK adds), with javascript:, data:, vbscript:, blob: and file: blocked.

What I found in review

The security core is right

Checking only event.source is not enough, because a frame can navigate to another origin while keeping the same window identity. Checking only event.origin is not enough when several frames share an origin. Pinning both, rejecting "*" and the opaque "null" origin, and comparing the full return URL rather than its origin close the forgery paths in the issue. I said so in the review.

The description promised things the code does not do

The pull request description documents public behavior changes that merchants and the changelog would rely on: a confirmation_in_progress error for a second confirm, payment_element_unmounted on unmount, a SDK_SESSION_TOKEN_DELIVERY_FAILED event, and resolving before a canceled beforeunload navigation. After the last commit at review time, none of these strings were in the diff, and a test comment said the opposite of the first one: "As on main, a later confirmation takes over the response listener". I asked for the description to match what ships, or for the missing commits to be pushed.

Split the security fix from the behavior changes

At review time the pull request was about 4,500 lines across about 90 files. The origin and source validation is self-contained: the security module, the handler guards and the 3-D Secure relay allowlist. The wallet results, overlay lifetime and polling changes are behavior changes with their own compatibility risk. In one pull request, a regression in the second group would block or revert the security fix with it.

What happened next

Takeaways

  • For postMessage, authenticate both event.source and event.origin, and send with an explicit target origin, never "*".
  • A pull request description is release-note material. If it describes behavior the code does not have, someone will build on it.
  • Ship security fixes on their own.

References

Code and history behind this post

More

Theme