fix: reject requests with # in request-target (#19830)

Author: sapphi-red

packages/vite/src/node/server/index.ts

@@ -98,6 +98,7 @@ import { transformRequest } from './transformRequest'
 import { searchForPackageRoot, searchForWorkspaceRoot } from './searchRoot'
 import type { DevEnvironment } from './environment'
 import { hostCheckMiddleware } from './middlewares/hostCheck'
+import { rejectInvalidRequestMiddleware } from './middlewares/rejectInvalidRequest'
 
 export interface ServerOptions extends CommonServerOptions {
   /**
@@ -864,6 +865,9 @@ export async function _createServer(
     middlewares.use(timeMiddleware(root))
   }
 
+  // disallows request that contains `#` in the URL
+  middlewares.use(rejectInvalidRequestMiddleware())
+
   // cors
   const { cors } = serverConfig
   if (cors !== false) {

packages/vite/src/node/server/middlewares/rejectInvalidRequest.ts

@@ -0,0 +1,20 @@
+import type { Connect } from 'dep-types/connect'
+
+export function rejectInvalidRequestMiddleware(): Connect.NextHandleFunction {
+  // Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
+  return function viteRejectInvalidRequestMiddleware(req, res, next) {
+    // HTTP spec does not allow `#` in the request-target
+    // (HTTP 1.1: https://datatracker.ietf.org/doc/html/rfc9112#section-3.2)
+    // (HTTP 2: https://datatracker.ietf.org/doc/html/rfc9113#section-8.3.1-2.4.1)
+    // But Node.js allows those requests.
+    // Our middlewares don't expect `#` to be included in `req.url`, especially the `server.fs.deny` checks.
+    if (req.url?.includes('#')) {
+      // HTTP 1.1 spec recommends sending 400 Bad Request
+      // (https://datatracker.ietf.org/doc/html/rfc9112#section-3.2-4)
+      res.writeHead(400)
+      res.end()
+      return
+    }
+    return next()
+  }
+}

playground/fs-serve/__tests__/fs-serve.spec.ts

@@ -1,3 +1,6 @@
+import net from 'node:net'
+import path from 'node:path'
+import { fileURLToPath } from 'node:url'
 import fetch from 'node-fetch'
 import {
   afterEach,
@@ -12,6 +15,8 @@ import WebSocket from 'ws'
 import testJSON from '../safe.json'
 import { browser, isServe, page, viteServer, viteTestUrl } from '~utils'
 
+const __dirname = path.dirname(fileURLToPath(import.meta.url))
+
 const getViteTestIndexHtmlUrl = () => {
   const srcPrefix = viteTestUrl.endsWith('/') ? '' : '/'
   // NOTE: viteTestUrl is set lazily
@@ -391,3 +396,73 @@ describe('cross origin', () => {
     )
   })
 })
+
+describe.runIf(isServe)('invalid request', () => {
+  const sendRawRequest = async (baseUrl: string, requestTarget: string) => {
+    return new Promise<string>((resolve, reject) => {
+      const parsedUrl = new URL(baseUrl)
+
+      const buf: Buffer[] = []
+      const client = net.createConnection(
+        { port: +parsedUrl.port, host: parsedUrl.hostname },
+        () => {
+          client.write(
+            [
+              `GET ${encodeURI(requestTarget)} HTTP/1.1`,
+              `Host: ${parsedUrl.host}`,
+              'Connection: Close',
+              '\r\n',
+            ].join('\r\n'),
+          )
+        },
+      )
+      client.on('data', (data) => {
+        buf.push(data)
+      })
+      client.on('end', (hadError) => {
+        if (!hadError) {
+          resolve(Buffer.concat(buf).toString())
+        }
+      })
+      client.on('error', (err) => {
+        reject(err)
+      })
+    })
+  }
+
+  const root = path
+    .resolve(__dirname.replace('playground', 'playground-temp'), '..')
+    .replace(/\\/g, '/')
+
+  test('request with sendRawRequest should work', async () => {
+    const response = await sendRawRequest(viteTestUrl, '/src/safe.txt')
+    expect(response).toContain('HTTP/1.1 200 OK')
+    expect(response).toContain('KEY=safe')
+  })
+
+  test('request with sendRawRequest should work with /@fs/', async () => {
+    const response = await sendRawRequest(
+      viteTestUrl,
+      path.posix.join('/@fs/', root, 'root/src/safe.txt'),
+    )
+    expect(response).toContain('HTTP/1.1 200 OK')
+    expect(response).toContain('KEY=safe')
+  })
+
+  test('should reject request that has # in request-target', async () => {
+    const response = await sendRawRequest(
+      viteTestUrl,
+      '/src/safe.txt#/../../unsafe.txt',
+    )
+    expect(response).toContain('HTTP/1.1 400 Bad Request')
+  })
+
+  test('should reject request that has # in request-target with /@fs/', async () => {
+    const response = await sendRawRequest(
+      viteTestUrl,
+      path.posix.join('/@fs/', root, 'root/src/safe.txt') +
+        '#/../../unsafe.txt',
+    )
+    expect(response).toContain('HTTP/1.1 400 Bad Request')
+  })
+})