router-server: guard upgrade proxy against absolute-url SSRF (#77) (#102)

* test: add rewrite request smuggling coverage

* router-server: guard upgrade proxy against absolute-url SSRF

* test: relax rewrite smuggling socket timeout

* router-server: preserve local websocket upgrades

(cherry picked from commit 846b7f825db2205c79e48b6fdc4acd8908235a43)

Author: ijjk

packages/next/src/server/lib/router-server.ts

@@ -817,21 +817,26 @@ export async function initialize(opts: {
           )
         },
       })
-      const { matchedOutput, parsedUrl } = await resolveRoutes({
-        req,
-        res,
-        isUpgradeReq: true,
-        signal: signalFromNodeResponse(socket),
-      })
+      const { finished, matchedOutput, parsedUrl, statusCode } =
+        await resolveRoutes({
+          req,
+          res,
+          isUpgradeReq: true,
+          signal: signalFromNodeResponse(socket),
+        })
 
       // TODO: allow upgrade requests to pages/app paths?
       // this was not previously supported
       if (matchedOutput) {
         return socket.end()
       }
 
-      if (parsedUrl.protocol) {
-        return await proxyRequest(req, socket, parsedUrl, head)
+      if (finished && parsedUrl.protocol) {
+        if (!statusCode) {
+          return await proxyRequest(req, socket, parsedUrl, head)
+        }
+
+        return socket.end()
       }
 
       // If there's no matched output, we don't handle the request as user's

test/e2e/rewrite-request-smuggling/rewrite-request-smuggling.test.ts

@@ -9,6 +9,9 @@ describe('rewrite-request-smuggling', () => {
     return
   }
 
+  const ssrfProbePath = '/secret-upgrade'
+  const ssrfProbeBody =
+    'SSRF_CONFIRMED: You reached the internal service at 127.0.0.1'
   let backend: http.Server
   let backendPort: number
   let intermediary: http.Server
@@ -56,11 +59,53 @@ describe('rewrite-request-smuggling', () => {
         socket.write(payload)
       })
       socket.once('error', reject)
-      socket.setTimeout(1000, () => socket.destroy())
+      socket.setTimeout(5000, () => socket.destroy())
       socket.once('close', () => resolve())
     })
   }
 
+  async function sendAbsoluteUrlUpgradePayload({
+    nextPort,
+    targetPort,
+  }: {
+    nextPort: number
+    targetPort: number
+  }) {
+    const payload = Buffer.from(
+      `GET http://127.0.0.1:${targetPort}${ssrfProbePath} HTTP/1.1\r\nHost: 127.0.0.1:${nextPort}\r\nConnection: Upgrade\r\nUpgrade: websocket\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n`,
+      'latin1'
+    )
+
+    return await new Promise<string>((resolve, reject) => {
+      const socket = net.createConnection({
+        host: '127.0.0.1',
+        port: nextPort,
+      })
+      const chunks: Buffer[] = []
+      let settled = false
+
+      const finish = () => {
+        if (settled) return
+        settled = true
+        resolve(Buffer.concat(chunks).toString('latin1'))
+      }
+
+      socket.once('connect', () => {
+        socket.write(payload)
+      })
+      socket.on('data', (chunk) => {
+        chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
+      })
+      socket.once('error', (err) => {
+        if (settled) return
+        settled = true
+        reject(err)
+      })
+      socket.setTimeout(1000, () => socket.destroy())
+      socket.once('close', finish)
+    })
+  }
+
   beforeAll(async () => {
     backendPort = await findPort()
     intermediaryPort = await findPort()
@@ -84,6 +129,14 @@ describe('rewrite-request-smuggling', () => {
       res.end('not-found')
     })
 
+    backend.on('upgrade', (req, socket) => {
+      backendRequests.push(`${req.method} ${req.url}`)
+      socket.write(
+        `HTTP/1.1 200 OK\r\nConnection: close\r\nContent-Type: text/plain\r\nContent-Length: ${Buffer.byteLength(ssrfProbeBody)}\r\n\r\n${ssrfProbeBody}`
+      )
+      socket.end()
+    })
+
     intermediary = http.createServer((req, res) => {
       const connectionHeader = Array.isArray(req.headers['connection'])
         ? req.headers['connection'].join(',')
@@ -231,4 +284,19 @@ describe('rewrite-request-smuggling', () => {
     })
     expect(backendRequests).not.toContain('GET /secret')
   })
+
+  it('does not proxy upgrade requests with absolute URLs without an external rewrite', async () => {
+    backendRequests.length = 0
+
+    const nextPort = Number(new URL(next.url).port)
+    const response = await sendAbsoluteUrlUpgradePayload({
+      nextPort,
+      targetPort: backendPort,
+    })
+
+    expect(response).not.toContain(ssrfProbeBody)
+    expect(
+      backendRequests.some((request) => request.includes(ssrfProbePath))
+    ).toBe(false)
+  })
 })