fix: validate return_to redirect URLs (#2532)

return_to query parameters are now parsed and only http and https URLs
are used for redirects, both in the frontend and when the createApp API
adds the connection parameters to the URL.

The production frontend build now also includes the same
Content-Security-Policy meta tag that is served as a header in http
mode, so the policy also applies where no HTTP headers are set, e.g. in
the desktop app.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

api/api.go

@@ -130,21 +130,7 @@ func (api *api) CreateApp(createAppRequest *CreateAppRequest) (*CreateAppRespons
 	responseBody.RelayUrls = relayUrls
 	responseBody.Lud16 = lightningAddress
 
-	if createAppRequest.ReturnTo != "" {
-		returnToUrl, err := url.Parse(createAppRequest.ReturnTo)
-		if err == nil {
-			query := returnToUrl.Query()
-			for _, relayUrl := range relayUrls {
-				query.Add("relay", relayUrl)
-			}
-			query.Add("pubkey", *app.WalletPubkey)
-			if lightningAddress != "" && !app.Isolated {
-				query.Add("lud16", lightningAddress)
-			}
-			returnToUrl.RawQuery = query.Encode()
-			responseBody.ReturnTo = returnToUrl.String()
-		}
-	}
+	responseBody.ReturnTo = buildReturnToUrl(createAppRequest.ReturnTo, relayUrls, *app.WalletPubkey, lightningAddress, app.Isolated)
 
 	var lud16 string
 	if lightningAddress != "" && !app.Isolated {
@@ -155,6 +141,28 @@ func (api *api) CreateApp(createAppRequest *CreateAppRequest) (*CreateAppRespons
 	return responseBody, nil
 }
 
+// buildReturnToUrl adds the connection query parameters to the return_to
+// URL the user will be redirected to. Only http and https URLs are accepted.
+func buildReturnToUrl(returnTo string, relayUrls []string, walletPubkey string, lightningAddress string, isolated bool) string {
+	if returnTo == "" {
+		return ""
+	}
+	returnToUrl, err := url.Parse(returnTo)
+	if err != nil || (returnToUrl.Scheme != "http" && returnToUrl.Scheme != "https") {
+		return ""
+	}
+	query := returnToUrl.Query()
+	for _, relayUrl := range relayUrls {
+		query.Add("relay", relayUrl)
+	}
+	query.Add("pubkey", walletPubkey)
+	if lightningAddress != "" && !isolated {
+		query.Add("lud16", lightningAddress)
+	}
+	returnToUrl.RawQuery = query.Encode()
+	return returnToUrl.String()
+}
+
 func (api *api) UpdateApp(userApp *db.App, updateAppRequest *UpdateAppRequest) error {
 	resolvedMaxAmountSat := ResolveToSat(updateAppRequest.MaxAmountSat, updateAppRequest.MaxAmountMsat, updateAppRequest.MaxAmount, nil)
 

api/apps_test.go

@@ -9,6 +9,32 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestBuildReturnToUrl(t *testing.T) {
+	relayUrls := []string{"wss://relay.getalby.com/v1"}
+	walletPubkey := "6f8bf1b7d58ac41b2c793837ba528c1d0a4a1cd2e3f5b7c9d0e1f2a3b4c5d6e7"
+
+	assert.Equal(t,
+		"https://example.com?pubkey=6f8bf1b7d58ac41b2c793837ba528c1d0a4a1cd2e3f5b7c9d0e1f2a3b4c5d6e7&relay=wss%3A%2F%2Frelay.getalby.com%2Fv1",
+		buildReturnToUrl("https://example.com", relayUrls, walletPubkey, "", false))
+
+	// existing query parameters are preserved and lud16 is added
+	assert.Equal(t,
+		"https://example.com/path?foo=bar&lud16=user%40getalby.com&pubkey=6f8bf1b7d58ac41b2c793837ba528c1d0a4a1cd2e3f5b7c9d0e1f2a3b4c5d6e7&relay=wss%3A%2F%2Frelay.getalby.com%2Fv1",
+		buildReturnToUrl("https://example.com/path?foo=bar", relayUrls, walletPubkey, "user@getalby.com", false))
+
+	// isolated apps do not receive a lightning address
+	assert.Equal(t,
+		"http://example.com?pubkey=6f8bf1b7d58ac41b2c793837ba528c1d0a4a1cd2e3f5b7c9d0e1f2a3b4c5d6e7&relay=wss%3A%2F%2Frelay.getalby.com%2Fv1",
+		buildReturnToUrl("http://example.com", relayUrls, walletPubkey, "user@getalby.com", true))
+
+	// only http and https URLs are accepted
+	assert.Equal(t, "", buildReturnToUrl("", relayUrls, walletPubkey, "", false))
+	assert.Equal(t, "", buildReturnToUrl("example.com/path", relayUrls, walletPubkey, "", false))
+	assert.Equal(t, "", buildReturnToUrl("example://app", relayUrls, walletPubkey, "", false))
+	assert.Equal(t, "", buildReturnToUrl("javascript:void(0)", relayUrls, walletPubkey, "", false))
+	assert.Equal(t, "", buildReturnToUrl("::invalid::", relayUrls, walletPubkey, "", false))
+}
+
 func TestCreateApp_SuperuserScopeIncorrectPassword(t *testing.T) {
 	cfg := mocks.NewMockConfig(t)
 	cfg.On("CheckUnlockPassword", "").Return(false)

frontend/src/screens/apps/NewApp.tsx

@@ -50,6 +50,7 @@ import {
 import { useApp } from "src/hooks/useApp";
 import { ConnectAppCard } from "src/screens/apps/ConnectAppCard";
 import { handleRequestError } from "src/utils/handleRequestError";
+import { safeReturnToUrl } from "src/utils/safeReturnToUrl";
 import Permissions from "../../components/Permissions";
 import { AppStoreApp } from "../../components/connections/SuggestedAppData";
 
@@ -87,7 +88,7 @@ const NewAppInternal = ({ capabilities }: NewAppInternalProps) => {
     appStoreApp?.firefoxLink;
 
   const pubkey = queryParams.get("pubkey") ?? "";
-  const returnTo = queryParams.get("return_to") ?? "";
+  const returnTo = safeReturnToUrl(queryParams.get("return_to")) ?? "";
 
   const nameParam = queryParams.get("name") || queryParams.get("c");
   const [appName, setAppName] = useState(nameParam || appStoreApp?.title || "");
@@ -307,10 +308,11 @@ const NewAppInternal = ({ capabilities }: NewAppInternalProps) => {
         );
       }
 
-      if (createAppResponse.returnTo) {
+      const returnToUrl = safeReturnToUrl(createAppResponse.returnTo);
+      if (returnToUrl) {
         // open connection URI directly in an app
         // eslint-disable-next-line react-hooks/immutability
-        window.location.href = createAppResponse.returnTo;
+        window.location.href = returnToUrl;
         return;
       }
       toast("App created");

frontend/src/screens/peers/ConnectPeer.tsx

@@ -9,6 +9,7 @@ import { Label } from "src/components/ui/label";
 import { splitSocketAddress } from "src/lib/utils";
 import { ConnectPeerRequest } from "src/types";
 import { request } from "src/utils/request";
+import { safeReturnToUrl } from "src/utils/safeReturnToUrl";
 
 export default function ConnectPeer() {
   const navigate = useNavigate();
@@ -18,7 +19,7 @@ export default function ConnectPeer() {
   const [connectionString, setConnectionString] = React.useState(
     queryParams.get("peer") ?? ""
   );
-  const returnTo = queryParams.get("return_to") ?? "";
+  const returnTo = safeReturnToUrl(queryParams.get("return_to")) ?? "";
 
   const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
     event.preventDefault();

frontend/src/utils/safeReturnToUrl.ts

@@ -0,0 +1,17 @@
+// Parses a return_to URL and only returns it if it is a
+// http or https URL. Relative URLs are resolved against the
+// current origin.
+export function safeReturnToUrl(returnTo: string | null): string | undefined {
+  if (!returnTo) {
+    return undefined;
+  }
+  try {
+    const url = new URL(returnTo, window.location.origin);
+    if (url.protocol === "http:" || url.protocol === "https:") {
+      return url.toString();
+    }
+  } catch {
+    // ignore invalid URLs
+  }
+  return undefined;
+}

frontend/vite.config.ts

@@ -50,7 +50,7 @@ export default defineConfig(({ command }) => ({
         maximumFileSizeToCacheInBytes: 3000000, // 3MB
       },
     }),
-    ...(command === "serve" ? [insertDevCSPPlugin] : []),
+    ...(command === "serve" ? [insertDevCSPPlugin] : [insertProdCSPPlugin]),
   ],
   server: {
     port: process.env.VITE_PORT ? parseInt(process.env.VITE_PORT) : undefined,
@@ -89,17 +89,39 @@ export default defineConfig(({ command }) => ({
 
 const DEVELOPMENT_NONCE = "'nonce-DEVELOPMENT'";
 
+// when making changes here, also update the CSP header in http_service.go
+const buildCSP = (nonce?: string) =>
+  `default-src 'self'${nonce ? " " + nonce : ""}; img-src 'self' https://uploads.getalby-assets.com https://cdn.getalby-assets.com https://getalby.com; connect-src 'self' https://api.getalby.com https://getalby.com https://zapplanner.albylabs.com wss://relay.getalby.com wss://relay2.getalby.com; frame-src https://www.youtube-nocookie.com`;
+
+const insertCSPMetaTag = (comment: string, csp: string) => (html: string) =>
+  html.replace(
+    "<head>",
+    `<head>
+        <!-- ${comment} -->
+        <meta http-equiv="Content-Security-Policy" content="${csp}" />`
+  );
+
 const insertDevCSPPlugin: Plugin = {
   name: "dev-csp",
   transformIndexHtml: {
     order: "pre",
-    handler: (html) => {
-      return html.replace(
-        "<head>",
-        `<head>
-        <!-- DEV-ONLY CSP - when making changes here, also update the CSP header in http_service.go (without the nonce!) -->
-        <meta http-equiv="Content-Security-Policy" content="default-src 'self' ${DEVELOPMENT_NONCE}; img-src 'self' https://uploads.getalby-assets.com https://cdn.getalby-assets.com https://getalby.com; connect-src 'self' https://api.getalby.com https://getalby.com https://zapplanner.albylabs.com wss://relay.getalby.com wss://relay2.getalby.com; frame-src https://www.youtube-nocookie.com" />`
-      );
-    },
+    handler: insertCSPMetaTag(
+      "DEV-ONLY CSP - when making changes here, also update the CSP header in http_service.go (without the nonce!)",
+      buildCSP(DEVELOPMENT_NONCE)
+    ),
+  },
+};
+
+// the same CSP is served as a HTTP header in http mode (see http_service.go).
+// The meta tag ensures the policy also applies where no HTTP headers are set,
+// e.g. in the desktop app.
+const insertProdCSPPlugin: Plugin = {
+  name: "prod-csp",
+  transformIndexHtml: {
+    order: "pre",
+    handler: insertCSPMetaTag(
+      "when making changes here, also update the CSP header in http_service.go",
+      buildCSP()
+    ),
   },
 };

http/http_service.go

@@ -66,6 +66,7 @@ func (httpSvc *HttpService) RegisterSharedRoutes(e *echo.Echo) {
 	e.Use(middleware.SecureWithConfig(middleware.SecureConfig{
 		ContentTypeNosniff:    "nosniff",
 		XFrameOptions:         "DENY",
+		// when making changes here, also update the CSP in frontend/vite.config.ts
 		ContentSecurityPolicy: "default-src 'self'; img-src 'self' https://uploads.getalby-assets.com https://cdn.getalby-assets.com https://getalby.com; connect-src 'self' https://api.getalby.com https://getalby.com https://zapplanner.albylabs.com wss://relay.getalby.com wss://relay2.getalby.com; frame-src https://www.youtube-nocookie.com",
 		ReferrerPolicy:        "no-referrer",
 	}))