From 76df6af936d5317d51ad717bd73fc53071f10cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ca=CC=81ssio=20Marcos=20Goulart?= Date: Wed, 11 Nov 2026 22:24:28 -0800 Subject: [PATCH] fix(discover): keep frames inside a page from opening links to other apps react-native-webview sent any link that isn't http(s) to the OS itself, before Discover's navigation handler saw it, wherever it loaded, so a frame embedded in a page could open a pairing link or another app without the user doing anything. Discover now asks the WebView for every URL and decides itself: pages and about:blank load; a link to another app loaded into the page itself still goes to the OS, so pairing from Discover is unchanged; the same link loaded into a frame inside the page is blocked. A background tab loads pages only. Android doesn't say which frame a URL loads into, so there every navigation counts as the page's own, as before. A link that can't be opened is logged with its scheme only, since it can carry a pairing URI. Co-Authored-By: Claude Opus 5.5 --- __tests__/helpers/browser.test.ts | 96 +++++++++++++++++++ .../DiscoveryScreen/DiscoveryScreen.tsx | 41 ++++---- .../components/WebViewContainer.tsx | 22 ++++- src/helpers/browser.ts | 70 ++++++++++++++ 4 files changed, 207 insertions(+), 22 deletions(-) diff --git a/__tests__/helpers/browser.test.ts b/__tests__/helpers/browser.test.ts index 4ec7fa8e1..07643298e 100644 --- a/__tests__/helpers/browser.test.ts +++ b/__tests__/helpers/browser.test.ts @@ -4,6 +4,7 @@ import { logger } from "config/logger"; import { isHomepageUrl, isDangerousScheme, + getDiscoverNavigationAction, getFaviconUrl, normalizeUrl, extractSearchQuery, @@ -11,8 +12,10 @@ import { generateTabId, clearAllCookies, clearAllWebViewData, + openLinkOutsideDiscover, } from "helpers/browser"; import { clearAllScreenshots } from "helpers/screenshots"; +import { Linking } from "react-native"; // Mock dependencies jest.mock("@react-native-cookies/cookies"); @@ -92,6 +95,99 @@ describe("browser helpers", () => { }); }); + describe("getDiscoverNavigationAction", () => { + const PAIRING_LINK = "freighterwallet://wc-redirect"; + const pairingUrl = `${PAIRING_LINK}?uri=wc%3Aabc%402`; + const action = (url: string, isTopFrame?: boolean) => + getDiscoverNavigationAction({ + url, + isTopFrame, + pairingLink: PAIRING_LINK, + }); + + it("loads http and https pages, in the page or in a frame", () => { + expect(action("https://example.com/", true)).toBe("load"); + expect(action("http://example.com/", false)).toBe("load"); + expect(action("https://example.com/", undefined)).toBe("load"); + }); + + it("loads about:blank, for empty frames", () => { + expect(action("about:blank", false)).toBe("load"); + }); + + it("blocks schemes that can run code", () => { + // eslint-disable-next-line no-script-url + expect(action("javascript:alert(1)", true)).toBe("block"); + expect(action("data:text/html,hi", true)).toBe("block"); + expect(action("blob:https://example.com/id", true)).toBe("block"); + }); + + it("blocks a page URL that carries the pairing link", () => { + expect(action(`https://example.com/?next=${pairingUrl}`, true)).toBe( + "block", + ); + }); + + it("opens a link to another app outside the WebView when the page itself follows it", () => { + expect(action(pairingUrl, true)).toBe("openOutside"); + expect(action("mailto:someone@example.com", true)).toBe("openOutside"); + }); + + it("opens it outside the WebView when the platform doesn't say which frame follows it", () => { + expect(action(pairingUrl, undefined)).toBe("openOutside"); + }); + + it("blocks a link to another app that a frame inside the page follows", () => { + expect(action(pairingUrl, false)).toBe("block"); + expect(action("mailto:someone@example.com", false)).toBe("block"); + }); + }); + + describe("openLinkOutsideDiscover", () => { + const otherWalletLink = + "otherwallet://wc?uri=wc%3Atopic%402%3FsymKey%3Dsecret"; + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("opens a link the OS can open", async () => { + jest.spyOn(Linking, "canOpenURL").mockResolvedValue(true); + const openURL = jest.spyOn(Linking, "openURL").mockResolvedValue(true); + + await openLinkOutsideDiscover(otherWalletLink); + + expect(openURL).toHaveBeenCalledWith(otherWalletLink); + }); + + it("doesn't open a link the OS says it can't", async () => { + jest.spyOn(Linking, "canOpenURL").mockResolvedValue(false); + const openURL = jest.spyOn(Linking, "openURL"); + + await openLinkOutsideDiscover(otherWalletLink); + + expect(openURL).not.toHaveBeenCalled(); + }); + + it("logs only the link's scheme when it can't be opened", async () => { + // React Native's error message carries the whole link. + jest + .spyOn(Linking, "canOpenURL") + .mockRejectedValue(new Error(`Unable to open URL: ${otherWalletLink}`)); + + await openLinkOutsideDiscover(otherWalletLink); + + expect(mockLogger.warn).toHaveBeenCalledWith( + "Discover", + "Failed to open a link outside Discover", + { scheme: "otherwallet" }, + ); + expect(JSON.stringify(mockLogger.warn.mock.calls)).not.toContain( + "secret", + ); + }); + }); + describe("getFaviconUrl", () => { it("should return empty string for homepage", () => { expect(getFaviconUrl("")).toBe(""); diff --git a/src/components/screens/DiscoveryScreen/DiscoveryScreen.tsx b/src/components/screens/DiscoveryScreen/DiscoveryScreen.tsx index a02b9c07f..aef88e038 100644 --- a/src/components/screens/DiscoveryScreen/DiscoveryScreen.tsx +++ b/src/components/screens/DiscoveryScreen/DiscoveryScreen.tsx @@ -18,9 +18,10 @@ import { useBrowserTabsStore } from "ducks/browserTabs"; import { WALLET_KIT_MT_REDIRECT_NATIVE } from "ducks/walletKit"; import { formatDisplayUrl, + getDiscoverNavigationAction, getFaviconUrl, - isDangerousScheme, isHomepageUrl, + openLinkOutsideDiscover, } from "helpers/browser"; import useAppTranslation from "hooks/useAppTranslation"; import { useBrowserActions } from "hooks/useBrowserActions"; @@ -37,6 +38,10 @@ import { } from "services/analytics/discover"; import { dataStorage } from "services/storage/storageFactory"; +/** A navigation Discover's WebView asks about. iOS says whether the page + * itself or a frame inside it navigates; Android leaves `isTopFrame` out. */ +type DiscoverNavigationRequest = WebViewNavigation & { isTopFrame?: boolean }; + type DiscoveryScreenProps = BottomTabScreenProps< MainTabStackParamList, typeof MAIN_TAB_ROUTES.TAB_DISCOVERY @@ -233,34 +238,32 @@ export const DiscoveryScreen: React.FC = () => { ); const handleShouldStartLoadWithRequest = useCallback( - (request: WebViewNavigation) => { + (request: DiscoverNavigationRequest) => { logger.debug( "WebViewContainer", "onShouldStartLoadWithRequest, request:", request, ); - // Block dangerous URL schemes that could execute arbitrary code - if (isDangerousScheme(request.url)) { - logger.debug( - "WebViewContainer", - "Blocked navigation to dangerous scheme:", - request.url, - ); - return false; - } + const action = getDiscoverNavigationAction({ + url: request.url, + isTopFrame: request.isTopFrame, + pairingLink: WALLET_KIT_MT_REDIRECT_NATIVE, + }); - // We should not handle WalletConnect URIs here - // let's handle them in the useWalletKitEventsManager hook instead - if (request.url.includes(WALLET_KIT_MT_REDIRECT_NATIVE)) { - logger.debug( + if (action === "openOutside") { + // Pairing links come back to the app through the deep-link handler + // (useWalletKitEventsManager), as other apps' links go to them. + openLinkOutsideDiscover(request.url); + } else if (action === "block" && request.isTopFrame === false) { + logger.warn( "WebViewContainer", - "onShouldStartLoadWithRequest, WalletConnect URI detected:", - request.url, + "Blocked a frame inside the page from opening a link outside Discover", + { scheme: request.url.split(":")[0] }, ); - return false; } - return true; + + return action === "load"; }, [], ); diff --git a/src/components/screens/DiscoveryScreen/components/WebViewContainer.tsx b/src/components/screens/DiscoveryScreen/components/WebViewContainer.tsx index e4708c020..a279b8b1f 100644 --- a/src/components/screens/DiscoveryScreen/components/WebViewContainer.tsx +++ b/src/components/screens/DiscoveryScreen/components/WebViewContainer.tsx @@ -7,7 +7,8 @@ import { } from "config/constants"; import { logger } from "config/logger"; import { useBrowserTabsStore } from "ducks/browserTabs"; -import { isDangerousScheme, isHomepageUrl } from "helpers/browser"; +import { WALLET_KIT_MT_REDIRECT_NATIVE } from "ducks/walletKit"; +import { getDiscoverNavigationAction, isHomepageUrl } from "helpers/browser"; import { pxValue } from "helpers/dimensions"; import { captureTabScreenshot } from "helpers/screenshots"; import useColors from "hooks/useColors"; @@ -20,7 +21,11 @@ import { WebView, WebViewNavigation } from "react-native-webview"; interface WebViewContainerProps { webViewRef: React.RefObject; onNavigationStateChange: (navState: WebViewNavigation) => void; - onShouldStartLoadWithRequest: (request: WebViewNavigation) => boolean; + // iOS says whether the page itself or a frame inside it navigates (see + // getDiscoverNavigationAction); Android leaves `isTopFrame` out. + onShouldStartLoadWithRequest: ( + request: WebViewNavigation & { isTopFrame?: boolean }, + ) => boolean; javaScriptEnabled?: boolean; domStorageEnabled?: boolean; } @@ -376,6 +381,10 @@ const WebViewContainer: React.FC = React.memo( } }} source={{ uri: tab.url }} + // Every URL reaches onShouldStartLoadWithRequest, links to + // other apps included. By default react-native-webview sends + // those to the OS itself, before any handler sees them. + originWhitelist={["*"]} onLoadEnd={() => handleLoadEnd(tab.id)} onScroll={() => handleScroll(tab.id)} allowsBackForwardNavigationGestures={isActive} @@ -385,7 +394,14 @@ const WebViewContainer: React.FC = React.memo( onShouldStartLoadWithRequest={ isActive ? onShouldStartLoadWithRequest - : (req) => !isDangerousScheme(req.url) + : // A tab in the background loads pages only, and + // opens nothing outside Discover. + (req) => + getDiscoverNavigationAction({ + url: req.url, + isTopFrame: req.isTopFrame, + pairingLink: WALLET_KIT_MT_REDIRECT_NATIVE, + }) === "load" } onLoadStart={() => handleWebViewMount(tab.id)} onError={() => handleWebViewUnmount(tab.id)} diff --git a/src/helpers/browser.ts b/src/helpers/browser.ts index 52353d8f0..6f7be8c66 100644 --- a/src/helpers/browser.ts +++ b/src/helpers/browser.ts @@ -3,6 +3,7 @@ import { BROWSER_CONSTANTS } from "config/constants"; import { logger } from "config/logger"; import { clearAllScreenshots } from "helpers/screenshots"; import { clearWebViewMediaConsent } from "helpers/webViewMediaConsent"; +import { Linking } from "react-native"; /** * Checks if the given URL is the homepage URL. @@ -28,6 +29,75 @@ export const isDangerousScheme = (url: string): boolean => { ); }; +/** What Discover's WebView does with a navigation. */ +export type DiscoverNavigationAction = "load" | "openOutside" | "block"; + +/** + * Decides what Discover's WebView does with a navigation. The WebView hands + * every URL to our handler, links to other apps included; react-native-webview + * used to send those to the OS itself, before our handler saw them. + * + * - `load`: an http(s) page, or `about:blank` for an empty frame. + * - `openOutside`: any other scheme loaded into the page itself goes to the + * OS, as before. That's how pairing from Discover works. + * - `block`: a scheme that can run code; a page URL that carries the wallet's + * pairing link; and any other scheme loaded into a frame inside the page, + * which could otherwise open an app, pairing included, without the user + * doing anything. + * + * `isTopFrame` says whether the navigation loads into the page itself or into + * a frame, not which of them started it: a link with `target="_top"` in a + * frame, which the user has to tap, counts as the page's. Android doesn't say + * (`isTopFrame` is undefined there), so there every navigation counts as the + * page's own, as before. + * + * @param navigation.url - The URL about to load + * @param navigation.isTopFrame - False when the URL loads into a frame inside the page + * @param navigation.pairingLink - The wallet's pairing link, e.g. `freighterwallet://wc-redirect` + * @returns What to do with the navigation + */ +export const getDiscoverNavigationAction = ({ + url, + isTopFrame, + pairingLink, +}: { + url: string; + isTopFrame?: boolean; + pairingLink: string; +}): DiscoverNavigationAction => { + if (isDangerousScheme(url)) { + return "block"; + } + + // What react-native-webview loads by default. + if (/^https?:\/\//.test(url) || url.startsWith("about:blank")) { + return url.includes(pairingLink) ? "block" : "load"; + } + + return isTopFrame === false ? "block" : "openOutside"; +}; + +/** + * Opens a link to another app from Discover the way react-native-webview did: + * only if the OS says it can. A failure is logged with the link's scheme + * only, since the link (and React Native's error message, which repeats it) + * can carry a pairing URI. + * + * @param url - The link to open + */ +export const openLinkOutsideDiscover = (url: string): Promise => + Linking.canOpenURL(url) + .then(async (canOpen) => { + if (canOpen) { + await Linking.openURL(url); + } + }) + .catch(() => { + logger.warn("Discover", "Failed to open a link outside Discover", { + scheme: url.split(":")[0], + }); + }); + /** * Generates a favicon URL from a website URL. * @param url - The website URL