Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allowed files logic (fix #5416) #5420

Merged
merged 9 commits into from Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 6 additions & 10 deletions packages/vite/src/node/plugins/resolve.ts
Expand Up @@ -24,7 +24,8 @@ import {
isDataUrl,
cleanUrl,
slash,
nestedResolveFrom
nestedResolveFrom,
isFileReadable
} from '../utils'
import { ViteDevServer, SSROptions } from '..'
import { createFilter } from '@rollup/pluginutils'
Expand Down Expand Up @@ -372,15 +373,10 @@ function tryResolveFile(
tryPrefix?: string,
skipPackageJson?: boolean
): string | undefined {
let isReadable = false
try {
// #2051 if we don't have read permission on a directory, existsSync() still
// works and will result in massively slow subsequent checks (which are
// unnecessary in the first place)
fs.accessSync(file, fs.constants.R_OK)
isReadable = true
} catch (e) {}
if (isReadable) {
// #2051 if we don't have read permission on a directory, existsSync() still
// works and will result in massively slow subsequent checks (which are
// unnecessary in the first place)
if (isFileReadable(file)) {
if (!fs.statSync(file).isDirectory()) {
return getRealPath(file, preserveSymlinks) + postfix
} else if (tryIndex) {
Expand Down
27 changes: 0 additions & 27 deletions packages/vite/src/node/server/middlewares/error.ts
Expand Up @@ -68,35 +68,8 @@ export function errorMiddleware(
if (allowNext) {
next()
} else {
if (err instanceof AccessRestrictedError) {
res.statusCode = 403
res.write(renderErrorHTML(err.message))
res.end()
}
res.statusCode = 500
res.end()
}
}
}

export class AccessRestrictedError extends Error {
constructor(msg: string) {
super(msg)
}
}

export function renderErrorHTML(msg: string): string {
// to have syntax highlighting and autocompletion in IDE
const html = String.raw
return html`
<body>
<h1>403 Restricted</h1>
<p>${msg.replace(/\n/g, '<br/>')}</p>
<style>
body {
padding: 1em 2em;
}
</style>
</body>
`
}
75 changes: 58 additions & 17 deletions packages/vite/src/node/server/middlewares/static.ts
@@ -1,4 +1,5 @@
import path from 'path'
import { ServerResponse } from 'http'
import sirv, { Options } from 'sirv'
import { Connect } from 'types/connect'
import { normalizePath, ViteDevServer } from '../..'
Expand All @@ -10,9 +11,9 @@ import {
isImportRequest,
isInternalRequest,
isWindows,
slash
slash,
isFileReadable
} from '../../utils'
import { AccessRestrictedError } from './error'

const sirvOptions: Options = {
dev: true,
Expand Down Expand Up @@ -51,11 +52,12 @@ export function serveStaticMiddleware(

// Keep the named function. The name is visible in debug logs via `DEBUG=connect:dispatcher ...`
return function viteServeStaticMiddleware(req, res, next) {
// only serve the file if it's not an html request
// only serve the file if it's not an html request or ends with `/`
// so that html requests can fallthrough to our html middleware for
// special processing
// also skip internal requests `/@fs/ /@vite-client` etc...
if (
req.url!.endsWith('/') ||
path.extname(cleanUrl(req.url!)) === '.html' ||
isInternalRequest(req.url!)
) {
Expand Down Expand Up @@ -86,8 +88,10 @@ export function serveStaticMiddleware(
if (resolvedUrl.endsWith('/') && !fileUrl.endsWith('/')) {
fileUrl = fileUrl + '/'
}
ensureServingAccess(fileUrl, server)

if (!ensureServingAccess(fileUrl, server, res, next)) {
return
}

if (redirected) {
req.url = redirected
}
Expand All @@ -110,7 +114,10 @@ export function serveRawFsMiddleware(
// searching based from fs root.
if (url.startsWith(FS_PREFIX)) {
// restrict files outside of `fs.allow`
ensureServingAccess(slash(path.resolve(fsPathFromId(url))), server)
if (!ensureServingAccess(slash(path.resolve(fsPathFromId(url))), server, res, next)) {
return
}

url = url.slice(FS_PREFIX.length)
if (isWindows) url = url.replace(/^[A-Z]:/i, '')

Expand All @@ -137,27 +144,61 @@ export function isFileServingAllowed(
return true

if (!server.config.server.fs.strict) {
server.config.logger.warnOnce(`Unrestricted file system access to "${url}"`)
server.config.logger.warnOnce(
`For security concerns, accessing files outside of serving allow list will ` +
if (isFileReadable(file)) {
server.config.logger.warnOnce(`Unrestricted file system access to "${url}"`)
server.config.logger.warnOnce(
`For security concerns, accessing files outside of serving allow list will ` +
`be restricted by default in the future version of Vite. ` +
`Refer to https://vitejs.dev/config/#server-fs-allow for more details.`
)
)
}
return true
}

return false
}

export function ensureServingAccess(url: string, server: ViteDevServer): void {
if (!isFileServingAllowed(url, server)) {
const allow = server.config.server.fs.allow
throw new AccessRestrictedError(
`The request url "${url}" is outside of Vite serving allow list:
function ensureServingAccess(
url: string,
server: ViteDevServer,
res: ServerResponse,
next: Connect.NextFunction,
): boolean {
if (isFileServingAllowed(url, server)) {
return true
}
if (isFileReadable(cleanUrl(url))) {
const message = `The request url "${url}" is outside of Vite serving allow list:

${allow.map((i) => `- ${i}`).join('\n')}
${server.config.server.fs.allow.map((i) => `- ${i}`).join('\n')}

Refer to docs https://vitejs.dev/config/#server-fs-allow for configurations and more details.`
)

server.config.logger.error(message + '\n')
res.statusCode = 403
res.write(renderRestrictedErrorHTML(message))
res.end()
}
else {
// if the file doesn't exist, we shouldn't restrict this path as it can
// be an API call. Middlewares would issue a 404 if the file isn't handled
next()
}
return false
}

function renderRestrictedErrorHTML(msg: string): string {
// to have syntax highlighting and autocompletion in IDE
const html = String.raw
return html`
<body>
<h1>403 Restricted</h1>
<p>${msg.replace(/\n/g, '<br/>')}</p>
<style>
body {
padding: 1em 2em;
}
</style>
</body>
`
}
5 changes: 2 additions & 3 deletions packages/vite/src/node/server/searchRoot.ts
@@ -1,6 +1,7 @@
import fs from 'fs'
import { dirname } from 'path'
import { join } from 'path'
import { isFileReadable } from '../utils'

// https://github.com/vitejs/vite/issues/2820#issuecomment-812495079
const ROOT_FILES = [
Expand All @@ -21,9 +22,7 @@ const ROOT_FILES = [
// yarn: https://classic.yarnpkg.com/en/docs/workspaces/#toc-how-to-use-it
function hasWorkspacePackageJSON(root: string): boolean {
const path = join(root, 'package.json')
try {
fs.accessSync(path, fs.constants.R_OK)
} catch {
if (!isFileReadable(path)) {
return false
}
const content = JSON.parse(fs.readFileSync(path, 'utf-8')) || {}
Expand Down
15 changes: 15 additions & 0 deletions packages/vite/src/node/utils.ts
Expand Up @@ -377,6 +377,21 @@ export function writeFile(
fs.writeFileSync(filename, content)
}

/**
* Use instead of fs.existsSync(filename)
* #2051 if we don't have read permission on a directory, existsSync() still
* works and will result in massively slow subsequent checks (which are
* unnecessary in the first place)
*/
export function isFileReadable(filename: string,) {
patak-dev marked this conversation as resolved.
Show resolved Hide resolved
try {
fs.accessSync(filename, fs.constants.R_OK)
return true
} catch {
return false
}
}

/**
* Delete every file and subdirectory. **The given directory must exist.**
* Pass an optional `skip` array to preserve files in the root directory.
Expand Down