Next.js: null origin can bypass Server Actions CSRF checks

@@ -616,9 +616,14 @@ export async function handleAction({
   workStore.fetchCache = 'default-no-store'
  
   const originHeader = req.headers['origin']
-  const originDomain =
-    typeof originHeader === 'string' && originHeader !== 'null'
-      ? new URL(originHeader).host
+  const originHost =
+    typeof originHeader === 'string'
+      ? // 'null' is a valid origin e.g. from privacy-sensitive contexts like sandboxed iframes.
+        // However, these contexts can still send along credentials like cookies,
+        // so we need to check if they're allowed cross-origin requests.
+        originHeader === 'null'
+        ? 'null'
+        : new URL(originHeader).host
       : undefined
   const host = parseHostHeader(req.headers)
  
@@ -631,15 +636,17 @@ export async function handleAction({
   }
   // This is to prevent CSRF attacks. If `x-forwarded-host` is set, we need to
   // ensure that the request is coming from the same host.
-  if (!originDomain) {
-    // This might be an old browser that doesn't send `host` header. We ignore
-    // this case.
+  if (!originHost) {
+    // This is a handcrafted request without an origin or a request from an unsafe browser.
+    // We'll let this through but log a warning.
+    // We can't guard against unsafe browsers and handcrafted requests can't contain
+    // user credentials that haven't been shared willingly.
     warning = 'Missing `origin` header from a forwarded Server Actions request.'
-  } else if (!host || originDomain !== host.value) {
+  } else if (!host || originHost !== host.value) {
     // If the customer sets a list of allowed origins, we'll allow the request.
     // These are considered safe but might be different from forwarded host set
     // by the infra (i.e. reverse proxies).
-    if (isCsrfOriginAllowed(originDomain, serverActions?.allowedOrigins)) {
+    if (isCsrfOriginAllowed(originHost, serverActions?.allowedOrigins)) {
       // Ignore it
     } else {
       if (host) {
@@ -650,7 +657,7 @@ export async function handleAction({
           }\` header with value \`${limitUntrustedHeaderValueForLogs(
             host.value
           )}\` does not match \`origin\` header with value \`${limitUntrustedHeaderValueForLogs(
-            originDomain
+            originHost
           )}\` from a forwarded Server Actions request. Aborting the action.`
         )
       } else {

@@ -26,17 +26,4 @@ describe('app-dir action allowed origins', () => {
       return await browser.elementByCss('#res').text()
     }, 'hi')
   })
- 
-  it('should not crash for requests from privacy sensitive contexts', async function () {
-    const res = await next.fetch('/', {
-      method: 'POST',
-      headers: {
-        Origin: 'null',
-        'Content-type': 'application/x-www-form-urlencoded',
-        'Sec-Fetch-Site': 'same-origin',
-      },
-    })
- 
-    expect({ status: res.status }).toEqual({ status: 200 })
-  })
 })

@@ -0,0 +1,71 @@
+import { nextTestSetup } from 'e2e-utils'
+import { retry } from 'next-test-utils'
+import { join } from 'path'
+ 
+describe('app-dir action allowed from opaque origins', () => {
+  const { next, skipped } = nextTestSetup({
+    files: join(__dirname, 'opaque-origin'),
+    skipDeployment: true,
+    env: {
+      NEXT_TEST_ALLOW_OPAQUE_ORIGIN: '1',
+    },
+  })
+ 
+  if (skipped) {
+    return
+  }
+ 
+  it('should succeed on submission', async function () {
+    const browser = await next.browser('/sandboxed')
+ 
+    await browser.elementByCss('input[type="submit"]').click()
+ 
+    await retry(async () => {
+      expect(await browser.elementByCss('output').text()).toEqual(
+        'Action Invoked'
+      )
+    })
+  })
+})
+ 
+describe('app-dir action disallowed from opaque origins', () => {
+  const { isNextDev, next, skipped } = nextTestSetup({
+    files: join(__dirname, 'opaque-origin'),
+    skipDeployment: true,
+    env: {
+      NEXT_TEST_ALLOW_OPAQUE_ORIGIN: '',
+    },
+  })
+ 
+  if (skipped) {
+    return
+  }
+ 
+  it('should fail on submission', async function () {
+    const browser = await next.browser('/sandboxed')
+    const beforeSubmissionLogOffset = (await browser.log()).length
+ 
+    await browser.elementByCss('input[type="submit"]').click()
+ 
+    await retry(async () => {
+      const logs = await browser.log()
+      const newLogs = logs.slice(beforeSubmissionLogOffset)
+      expect(newLogs).toEqual(
+        expect.arrayContaining([
+          {
+            source: 'error',
+            message:
+              'Failed to load resource: the server responded with a status of 500 (Internal Server Error)',
+          },
+        ])
+      )
+    })
+    if (isNextDev) {
+      // page is borked at this point. Nothing interesting to assert on.
+    } else {
+      expect(await browser.elementByCss('body').text()).toEqual(
+        'Internal Server Error'
+      )
+    }
+  })
+})

@@ -0,0 +1,10 @@
+'use server'
+ 
+import { cookies } from 'next/headers'
+ 
+export async function log() {
+  console.log('action invoked')
+  const cookieStore = await cookies()
+  cookieStore.set('log-action-invoked', '1')
+  return 'hi'
+}

@@ -0,0 +1,21 @@
+import { Suspense } from 'react'
+ 
+export default function RootLayout({ children }) {
+  return (
+    // Needs to be above html since we can't allow scripts in sandbox
+    <Suspense fallback={<div>Loading...</div>}>
+      <html>
+        <head />
+        <body>
+          <ul>
+            {/* These need to be MPAs so that the appropriate headers are applied */}
+            <li>
+              <a href="/sandboxed">Sandboxed Page</a>
+            </li>
+          </ul>
+          {children}
+        </body>
+      </html>
+    </Suspense>
+  )
+}

@@ -0,0 +1,14 @@
+import { cookies } from 'next/headers'
+import { log } from '../action'
+ 
+export default async function Page() {
+  const cookieStore = await cookies()
+  const cookie = cookieStore.get('log-action-invoked')
+  const hasLogged = cookie?.value === '1'
+  return (
+    <form action={log}>
+      <input type="submit" />
+      <output>{hasLogged ? 'Action Invoked' : 'Action Not Invoked'}</output>
+    </form>
+  )
+}

@@ -0,0 +1,27 @@
+const allowOpaqueOrigin = process.env.NEXT_TEST_ALLOW_OPAQUE_ORIGIN === '1'
+ 
+/** @type {import('next').NextConfig} */
+module.exports = {
+  productionBrowserSourceMaps: true,
+  logging: {
+    fetches: {},
+  },
+  headers() {
+    return [
+      {
+        source: '/sandboxed',
+        headers: [
+          {
+            key: 'Content-Security-Policy',
+            value: 'sandbox allow-forms',
+          },
+        ],
+      },
+    ]
+  },
+  experimental: {
+    serverActions: {
+      allowedOrigins: allowOpaqueOrigin ? ['null'] : [],
+    },
+  },
+}