From 5dba5471d176202042758fcfad4f7fb4378d996a Mon Sep 17 00:00:00 2001
From: Valentin Brandl <mail@vbrandl.net>
Date: Fri, 16 Aug 2024 16:34:43 +0200
Subject: [PATCH] Reject illegal names

---
 src/index.test.ts          | 16 ++++++++++++++--
 src/lib/index.ts           | 17 +++++++++++++++--
 src/routes/+page.server.ts |  7 ++++++-
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/index.test.ts b/src/index.test.ts
index 68c8470..474ce58 100644
--- a/src/index.test.ts
+++ b/src/index.test.ts
@@ -2,7 +2,19 @@ import safePath from '$lib';
 import { describe, it, expect } from 'vitest';
 
 describe('safe path', () => {
-	it('removes non alphanum from string', () => {
-		expect(safePath('../../!=-.,/abc123')).toBe('abc123');
+	it('reject names with ../', () => {
+		expect(safePath('../foobar', './uploads')).toBe(false);
+	});
+
+	it('accept names with ./', () => {
+		expect(safePath('./foobar', './uploads')).toBe(true);
+	});
+
+	it('reject names with /', () => {
+		expect(safePath('foo/bar', './uploads')).toBe(false);
+	});
+
+	it('accept happy path', () => {
+		expect(safePath('foobar', './uploads')).toBe(false);
 	});
 });
diff --git a/src/lib/index.ts b/src/lib/index.ts
index 1df0551..f9dbc45 100644
--- a/src/lib/index.ts
+++ b/src/lib/index.ts
@@ -1,10 +1,23 @@
 // place files you want to import through the `$lib` alias in this folder.
+import path from 'path';
 
-const safePath = (input: string) => input.replace(/\W/g, '');
+function safePath(name: string, basePath: string): boolean {
+	const fullPath = `${basePath}/${name}`;
+	const relative = path.relative(basePath, fullPath);
+	return (
+		!!relative &&
+		// does move out of `basePath`
+		!relative.startsWith('..') &&
+		// exactly one layer deep, e.g. no `./uplodas/foo/bar`
+		!relative.includes('/') &&
+		// result is not an absolute path
+		!path.isAbsolute(relative)
+	);
+}
 
 const defaultPath: string = './uploads';
 if (!('STORAGE_PATH' in process.env)) {
-	console.log(`'STORAGE_PATH' environment variable is not set. Defaulting to ${defaultPath}`);
+	console.warn(`'STORAGE_PATH' environment variable is not set. Defaulting to ${defaultPath}`);
 }
 export const storagePath: string = process.env.STORAGE_PATH ?? defaultPath;
 
diff --git a/src/routes/+page.server.ts b/src/routes/+page.server.ts
index 0ad06d6..082f427 100644
--- a/src/routes/+page.server.ts
+++ b/src/routes/+page.server.ts
@@ -34,7 +34,12 @@ export const actions = {
 			return fail(400, { field: 'name', name: formName, incorrect: true });
 		}
 
-		const name = safePath(formName as string);
+		const name = formName as string;
+
+		if (!safePath(name, storagePath)) {
+			return fail(400, { field: 'name', name: name, incorrect: true });
+		}
+		// const name = safePath(formName as string);
 
 		files.forEach(async (file) => {
 			const outPath = `${storagePath}/${name}`;