fix: validate schemeful action origins (#15420)

Assisted-By: devx/597728f0-1b3f-4153-b929-18dbff6cd683

Author: brophdawg11

packages/react-router/.changes/patch.schemeful-action-origins.md

@@ -0,0 +1 @@
+Improve validation of action request origins

packages/react-router/__tests__/server-runtime/actions-test.ts

@@ -41,9 +41,23 @@ describe("throwIfPotentialCSRFAttack", () => {
         },
       });
       expect(() => throwIfPotentialCSRFAttack(request, undefined)).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
+
+    it("should compare complete origins", () => {
+      for (let [origin, requestUrl] of [
+        ["http://example.com", "https://example.com/action"],
+        ["https://example.com", "http://example.com/action"],
+      ]) {
+        let request = new Request(requestUrl, {
+          method: "POST",
+          headers: { origin },
+        });
+
+        expect(() => throwIfPotentialCSRFAttack(request, undefined)).toThrow();
+      }
+    });
   });
 
   describe("with allowed origins", () => {
@@ -59,6 +73,18 @@ describe("throwIfPotentialCSRFAttack", () => {
       ).not.toThrow();
     });
 
+    it("should support explicitly allowed hosts", () => {
+      let request = new Request("https://example.com/action", {
+        method: "POST",
+        headers: {
+          origin: "http://example.com",
+        },
+      });
+      expect(() =>
+        throwIfPotentialCSRFAttack(request, ["example.com"]),
+      ).not.toThrow();
+    });
+
     it("should not throw when origin matches a wildcard pattern", () => {
       let request = new Request("https://example.com/action", {
         method: "POST",
@@ -93,7 +119,7 @@ describe("throwIfPotentialCSRFAttack", () => {
       expect(() =>
         throwIfPotentialCSRFAttack(request, ["trusted.com", "*.safe.com"]),
       ).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 
@@ -134,7 +160,7 @@ describe("throwIfPotentialCSRFAttack", () => {
         },
       });
       expect(() => throwIfPotentialCSRFAttack(request, undefined)).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 
@@ -181,7 +207,7 @@ describe("throwIfPotentialCSRFAttack", () => {
         },
       });
       expect(() => throwIfPotentialCSRFAttack(request, undefined)).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 
@@ -195,7 +221,7 @@ describe("throwIfPotentialCSRFAttack", () => {
       expect(() =>
         throwIfPotentialCSRFAttack(request, ["", "other.com"]),
       ).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 
@@ -219,7 +245,7 @@ describe("throwIfPotentialCSRFAttack", () => {
         },
       });
       expect(() => throwIfPotentialCSRFAttack(request, undefined)).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 
@@ -243,7 +269,7 @@ describe("throwIfPotentialCSRFAttack", () => {
         },
       });
       expect(() => throwIfPotentialCSRFAttack(request, ["*"])).toThrow(
-        "`request.url` host does not match `origin` header from a forwarded action request",
+        "`request.url` origin does not match `origin` header from a forwarded action request",
       );
     });
 

packages/react-router/lib/actions.ts

@@ -4,24 +4,30 @@ export function throwIfPotentialCSRFAttack(
 ) {
   let originHeader = request.headers.get("origin");
   let originDomain: string | null = null;
+  let originUrl: URL | null = null;
 
   try {
-    originDomain =
-      typeof originHeader === "string" && originHeader !== "null"
-        ? new URL(originHeader).host
-        : originHeader;
+    if (typeof originHeader === "string" && originHeader !== "null") {
+      originUrl = new URL(originHeader);
+      originDomain = originUrl.host;
+    } else {
+      originDomain = originHeader;
+    }
   } catch {
     throw new Error(
       `\`origin\` header is not a valid URL. Aborting the action.`,
     );
   }
-  let host = new URL(request.url).host;
+  let requestUrl = new URL(request.url);
+  let originMatchesRequest = originUrl
+    ? originUrl.origin === requestUrl.origin
+    : originDomain === requestUrl.host;
 
-  if (originDomain && originDomain !== host) {
+  if (originDomain && !originMatchesRequest) {
     if (!isAllowedOrigin(originDomain, allowedActionOrigins)) {
       // This seems to be an CSRF attack. We should not proceed with the action.
       throw new Error(
-        "The `request.url` host does not match `origin` header from a forwarded " +
+        "The `request.url` origin does not match `origin` header from a forwarded " +
           "action request. Aborting the action.",
       );
     }