fix: restrict static middleware fs access (#5361)

vitejs · Oct 23, 2021 · 1f4723b · 1f4723b
1 parent 7970239
commit 1f4723b
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 34 deletions.
diff --git a/packages/playground/fs-serve/__tests__/fs-serve.spec.ts b/packages/playground/fs-serve/__tests__/fs-serve.spec.ts
@@ -3,30 +3,47 @@ import { isBuild } from '../../testUtils'
 const json = require('../safe.json')
 const stringified = JSON.stringify(json)
 
-if (!isBuild) {
-  test('default import', async () => {
-    expect(await page.textContent('.full')).toBe(stringified)
+describe('main', () => {
+  beforeAll(async () => {
+    // viteTestUrl is globally injected in scripts/jestPerTestSetup.ts
+    await page.goto(viteTestUrl + '/src/')
   })
 
-  test('named import', async () => {
-    expect(await page.textContent('.named')).toBe(json.msg)
-  })
+  if (!isBuild) {
+    test('default import', async () => {
+      expect(await page.textContent('.full')).toBe(stringified)
+    })
 
-  test('safe fetch', async () => {
-    expect(await page.textContent('.safe-fetch')).toBe(stringified)
-    expect(await page.textContent('.safe-fetch-status')).toBe('200')
-  })
+    test('named import', async () => {
+      expect(await page.textContent('.named')).toBe(json.msg)
+    })
 
-  test('unsafe fetch', async () => {
-    expect(await page.textContent('.unsafe-fetch')).toBe('')
-    expect(await page.textContent('.unsafe-fetch-status')).toBe('403')
-  })
+    test('safe fetch', async () => {
+      expect(await page.textContent('.safe-fetch')).toMatch('KEY=safe')
+      expect(await page.textContent('.safe-fetch-status')).toBe('200')
+    })
 
-  test('nested entry', async () => {
-    expect(await page.textContent('.nested-entry')).toBe('foobar')
-  })
-} else {
-  test('dummy test to make jest happy', async () => {
-    // Your test suite must contain at least one test.
-  })
-}
+    test('unsafe fetch', async () => {
+      expect(await page.textContent('.unsafe-fetch')).toMatch('403 Restricted')
+      expect(await page.textContent('.unsafe-fetch-status')).toBe('403')
+    })
+
+    test('safe fs fetch', async () => {
+      expect(await page.textContent('.safe-fs-fetch')).toBe(stringified)
+      expect(await page.textContent('.safe-fs-fetch-status')).toBe('200')
+    })
+
+    test('unsafe fs fetch', async () => {
+      expect(await page.textContent('.unsafe-fs-fetch')).toBe('')
+      expect(await page.textContent('.unsafe-fs-fetch-status')).toBe('403')
+    })
+
+    test('nested entry', async () => {
+      expect(await page.textContent('.nested-entry')).toBe('foobar')
+    })
+  } else {
+    test('dummy test to make jest happy', async () => {
+      // Your test suite must contain at least one test.
+    })
+  }
+})
diff --git a/packages/playground/fs-serve/root/.env b/packages/playground/fs-serve/root/.env
@@ -0,0 +1 @@
+KEY=unsafe
diff --git a/packages/playground/fs-serve/root/src/.env b/packages/playground/fs-serve/root/src/.env
@@ -0,0 +1 @@
+KEY=safe
diff --git a/packages/playground/fs-serve/root/index.html → ...s/playground/fs-serve/root/src/index.html b/packages/playground/fs-serve/root/index.html → ...s/playground/fs-serve/root/src/index.html
@@ -1,3 +1,5 @@
+<link rel="icon" href="/src/favicon.ico" />
+
 <h2>Normal Import</h2>
 <pre class="full"></pre>
 <pre class="named"></pre>
@@ -10,34 +12,65 @@ <h2>Unsafe Fetch</h2>
 <pre class="unsafe-fetch-status"></pre>
 <pre class="unsafe-fetch"></pre>
 
+<h2>Safe /@fs/ Fetch</h2>
+<pre class="safe-fs-fetch-status"></pre>
+<pre class="safe-fs-fetch"></pre>
+
+<h2>Unsafe /@fs/ Fetch</h2>
+<pre class="unsafe-fs-fetch-status"></pre>
+<pre class="unsafe-fs-fetch"></pre>
+
 <h2>Nested Entry</h2>
 <pre class="nested-entry"></pre>
 
 <script type="module">
-  import '../entry'
-  import json, { msg } from '../safe.json'
+  import '../../entry'
+  import json, { msg } from '../../safe.json'
 
   text('.full', JSON.stringify(json))
   text('.named', msg)
 
+  // inside allowed dir, safe fetch
+  fetch('/src/.env')
+    .then((r) => {
+      text('.safe-fetch-status', r.status)
+      return r.text()
+    })
+    .then((data) => {
+      text('.safe-fetch', JSON.stringify(data))
+    })
+
+  // outside of allowed dir, treated as unsafe
+  fetch('/.env')
+    .then((r) => {
+      text('.unsafe-fetch-status', r.status)
+      return r.text()
+    })
+    .then((data) => {
+      text('.unsafe-fetch', data)
+    })
+    .catch((e) => {
+      console.error(e)
+    })
+
   // imported before, should be treated as safe
   fetch('/@fs/' + ROOT + '/safe.json')
     .then((r) => {
-      text('.safe-fetch-status', r.status)
+      text('.safe-fs-fetch-status', r.status)
       return r.json()
     })
     .then((data) => {
-      text('.safe-fetch', JSON.stringify(data))
+      text('.safe-fs-fetch', JSON.stringify(data))
     })
 
   // not imported before, outside of root, treated as unsafe
   fetch('/@fs/' + ROOT + '/unsafe.json')
     .then((r) => {
-      text('.unsafe-fetch-status', r.status)
+      text('.unsafe-fs-fetch-status', r.status)
       return r.json()
     })
     .then((data) => {
-      text('.unsafe-fetch', JSON.stringify(data))
+      text('.unsafe-fs-fetch', JSON.stringify(data))
     })
     .catch((e) => {
       console.error(e)
@@ -46,4 +79,4 @@ <h2>Nested Entry</h2>
   function text(sel, text) {
     document.querySelector(sel).textContent = text
   }
-</script>
+</script>
diff --git a/packages/playground/fs-serve/root/vite.config.js b/packages/playground/fs-serve/root/vite.config.js
@@ -4,10 +4,17 @@ const path = require('path')
  * @type {import('vite').UserConfig}
  */
 module.exports = {
+  build: {
+    rollupOptions: {
+      input: {
+        main: path.resolve(__dirname, 'src/index.html')
+      }
+    }
+  },
   server: {
     fs: {
       strict: true,
-      allow: [__dirname]
+      allow: [path.resolve(__dirname, 'src')]
     },
     hmr: {
       overlay: false

diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts
@@ -525,7 +525,7 @@ export async function createServer(
 
   // serve static files
   middlewares.use(serveRawFsMiddleware(server))
-  middlewares.use(serveStaticMiddleware(root, config))
+  middlewares.use(serveStaticMiddleware(root, server))
 
   // spa fallback
   if (!middlewareMode || middlewareMode === 'html') {

diff --git a/packages/vite/src/node/server/middlewares/static.ts b/packages/vite/src/node/server/middlewares/static.ts
@@ -1,7 +1,7 @@
 import path from 'path'
 import sirv, { Options } from 'sirv'
 import { Connect } from 'types/connect'
-import { normalizePath, ResolvedConfig, ViteDevServer } from '../..'
+import { normalizePath, ViteDevServer } from '../..'
 import { FS_PREFIX } from '../../constants'
 import {
   cleanUrl,
@@ -45,7 +45,7 @@ export function servePublicMiddleware(dir: string): Connect.NextHandleFunction {
 
 export function serveStaticMiddleware(
   dir: string,
-  config: ResolvedConfig
+  server: ViteDevServer
 ): Connect.NextHandleFunction {
   const serve = sirv(dir, sirvOptions)
 
@@ -66,7 +66,7 @@ export function serveStaticMiddleware(
 
     // apply aliases to static requests as well
     let redirected: string | undefined
-    for (const { find, replacement } of config.resolve.alias) {
+    for (const { find, replacement } of server.config.resolve.alias) {
       const matches =
         typeof find === 'string' ? url.startsWith(find) : find.test(url)
       if (matches) {
@@ -79,6 +79,16 @@ export function serveStaticMiddleware(
       if (redirected.startsWith(dir)) {
         redirected = redirected.slice(dir.length)
       }
+    }
+
+    const resolvedUrl = redirected || url
+    let fileUrl = path.resolve(dir, resolvedUrl.replace(/^\//, ''))
+    if (resolvedUrl.endsWith('/') && !fileUrl.endsWith('/')) {
+      fileUrl = fileUrl + '/'
+    }
+    ensureServingAccess(fileUrl, server)
+
+    if (redirected) {
       req.url = redirected
     }