require a separator when checking a path is inside its root
resolveUserPath and five sibling checks used startsWith(rootDir), which also accepts a sibling directory whose name begins with the root's: from a root of /home/br, "../br-backup/secret" resolves to /home/br-backup/secret and passed. Compare against root + sep (or the root itself) via a shared isInside helper. Verified the escape cases now deny while "", ".", and ordinary relative paths still resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c13875cef8
commit
78130f21ce
@@ -1,5 +1,5 @@
|
||||
import { createRouter } from '@@/create-router';
|
||||
import { resolve, dirname, join, parse as parsePath } from 'node:path';
|
||||
import { resolve, dirname, join, sep, parse as parsePath } from 'node:path';
|
||||
import { readdir, stat, mkdir, rm, rename, readFile, cp } from 'node:fs/promises';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { getOwnerHomeDir, DATA_PATH } from '@@/data-path';
|
||||
@@ -69,9 +69,14 @@ function getRootDir(user: UserCtx, root?: string): string {
|
||||
throw errors.BAD_REQUEST(`Invalid root: ${root}`);
|
||||
}
|
||||
|
||||
// A resolved path counts as contained only when it IS the root or sits beneath it. A bare
|
||||
// startsWith also accepts a sibling whose name merely begins with the root's — `/home/br-backup`
|
||||
// passes a `/home/br` check — which is how `..` segments escaped.
|
||||
const isInside = (root: string, target: string): boolean => target === root || target.startsWith(root + sep);
|
||||
|
||||
function resolveUserPath(rootDir: string, relPath: string): string {
|
||||
const resolved = resolve(rootDir, relPath.replace(/^\/+/, ''));
|
||||
if (!resolved.startsWith(rootDir)) throw errors.FORBIDDEN('Path outside root directory');
|
||||
if (!isInside(rootDir, resolved)) throw errors.FORBIDDEN('Path outside root directory');
|
||||
return resolved;
|
||||
}
|
||||
|
||||
@@ -176,7 +181,7 @@ router.get('/ls', async (ctx) => {
|
||||
names.map(async (name) => {
|
||||
const fullPath = resolve(absPath, name);
|
||||
// Skip entries that escape the home dir (shouldn't happen but be safe)
|
||||
if (!fullPath.startsWith(rootDir)) return null;
|
||||
if (!isInside(rootDir, fullPath)) return null;
|
||||
const s = await stat(fullPath).catch(() => null);
|
||||
if (!s) return null;
|
||||
return {
|
||||
@@ -255,7 +260,7 @@ router.post('/upload', async (ctx) => {
|
||||
for (const file of files) {
|
||||
if (!(file instanceof File)) continue;
|
||||
const filePath = resolve(targetDir, file.name);
|
||||
if (!filePath.startsWith(rootDir)) continue;
|
||||
if (!isInside(rootDir, filePath)) continue;
|
||||
await mkdir(dirname(filePath), { recursive: true });
|
||||
await Bun.write(filePath, file);
|
||||
}
|
||||
@@ -275,7 +280,7 @@ router.post('/rename', async (ctx) => {
|
||||
if (absPath === rootDir) throw errors.FORBIDDEN('Cannot rename home directory');
|
||||
|
||||
const newPath = resolve(dirname(absPath), newName);
|
||||
if (!newPath.startsWith(rootDir)) throw errors.FORBIDDEN('Path outside home directory');
|
||||
if (!isInside(rootDir, newPath)) throw errors.FORBIDDEN('Path outside home directory');
|
||||
|
||||
await rename(absPath, newPath);
|
||||
return ctx.json({ ok: true });
|
||||
@@ -644,7 +649,7 @@ router.post('/save-result', async (ctx) => {
|
||||
|
||||
const homeDir = getRootDir(user, 'home');
|
||||
const destAbs = resolve(homeDir, relativePath);
|
||||
if (!destAbs.startsWith(homeDir)) throw errors.FORBIDDEN('Path outside home directory');
|
||||
if (!isInside(homeDir, destAbs)) throw errors.FORBIDDEN('Path outside home directory');
|
||||
|
||||
await mkdir(dirname(destAbs), { recursive: true });
|
||||
await cp(srcAbs, destAbs);
|
||||
@@ -1084,7 +1089,7 @@ router.get('/search', async (ctx) => {
|
||||
for (const name of names) {
|
||||
if (results.length >= MAX_RESULTS) break;
|
||||
const fullPath = resolve(dir, name);
|
||||
if (!fullPath.startsWith(rootDir)) continue;
|
||||
if (!isInside(rootDir, fullPath)) continue;
|
||||
const s = await stat(fullPath).catch(() => null);
|
||||
if (!s) continue;
|
||||
if (name.toLowerCase().includes(query)) {
|
||||
|
||||
Reference in New Issue
Block a user