fix(security): prevent trezor-connect-src connectSrc-override XSS (Immunefi #40463) - #1143
Merged
Conversation
…act key @trezor/connect-web parseConnectSettings picks up connectSrc from any &-segment whose text contains the substring trezor-connect-src (indexOf >= 0), then uses it verbatim as the popup/iframe URL with no scheme validation for non-http values, so a javascript: value executes in our origin (Immunefi #40463). The previous guard used URLSearchParams.has(trezor-connect-src), an exact-key match, trivially bypassed by variants like trezor-connect-srcz or x-trezor-connect-src that the library still matches by substring. Replace it with stripTrezorConnectSrcParam(), which removes any query segment containing the substring case-insensitively (a superset of connect-web case-sensitive matching), before TrezorConnect.init() can read the query string. Unit-tested: exact param, the -srcz / x- prefixed / mixed-case bypasses, multi-param preservation, and no-op cases.
✅ Deploy Preview for sovryn-dapp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DOM-based reflected XSS via the
@trezor/connect-webconnectSrcoverride.parseConnectSettingspicks up any query-string segment whose text contains the substringtrezor-connect-srcand uses its value verbatim as the Trezor popup/iframe URL, with no scheme check for non-httpvalues. A?…trezor-connect-src…=javascript:…payload therefore executes in our origin once the Trezor connect flow is launched.Reported and bountied via Immunefi #40463 and previously patched with an exact-key filter (
URLSearchParams.has('trezor-connect-src')) — which is trivially bypassed by appending any character (trezor-connect-srcz,x-trezor-connect-src, …), since the library matches by substring, not exact key.Fix
Replace the exact-key guard with
stripTrezorConnectSrcParam(), which strips any query segment containing the substring (case-insensitively — a superset of the library's case-sensitive match) beforeTrezorConnect.init()can read the query string. It only rewrites the URL when a match is present, preserving path, other query params, and#fragmentotherwise.Verification
sanitizeUrl.test.ts): exact param,-srcz/x-prefixed / mixed-case bypasses, targeted multi-param removal, and no-op cases.-srczpayload and the original param are both stripped on load; walking Get started → Trezor → Continue,window.opentargetshttps://connect.trezor.io/9/popup.html(neverjavascript:), no alert fires. Legit links, paths, query params, and#fragmentsare preserved.Scope
App-layer fix only. A CSP backstop and the
@trezor/connect-webupgrade (the Trezor popup currently shows a "deprecated version" notice for the pinned9.1.4) are tracked separately.