From 6b65fda69df4582239abd94587fa5c1fcaf7b0fd Mon Sep 17 00:00:00 2001 From: eternal-flame-AD Date: Wed, 24 Jul 2024 12:59:48 -0500 Subject: [PATCH] implement safe path handling Signed-off-by: eternal-flame-AD --- src/index.test.js | 24 +++++++++++++++++++ src/lib/server/database/loadData.ts | 8 ++++++- src/lib/server/safePath.ts | 24 +++++++++++++++++++ src/routes/api/database/song/[id]/+server.ts | 8 ++++++- src/routes/database/edit/[id]/+page.server.ts | 8 ++++++- 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 src/lib/server/safePath.ts diff --git a/src/index.test.js b/src/index.test.js index 2bf1e23..1c3f282 100644 --- a/src/index.test.js +++ b/src/index.test.js @@ -1,5 +1,7 @@ +import path from 'path'; import { describe, it, expect } from 'vitest'; import formatDuration from '$lib/formatDuration'; +import { safePath } from '$lib/server/safePath'; describe('formatDuration test', () => { it('converts 120 seconds to "2:00"', () => { @@ -21,4 +23,26 @@ describe('formatDuration test', () => { it('converts 3601 seconds to "1:00:01"', () => { expect(formatDuration(3601)).toBe('1:00:01'); }); +}); + +describe('safePath test', () => { + const base = "data/subdir"; + it('rejects empty string', () => { + expect(safePath('', { base })).toBe(null); + }); + it('accepts a regular path', () => { + expect(safePath('subsubdir/file.txt', { base })).toBe('data/subdir/subsubdir/file.txt'); + }); + it('rejects path with ..', () => { + expect(safePath('../file.txt', { base })).toBe(null); + }); + it('accepts path with .', () => { + expect(safePath('./file.txt', { base })).toBe('data/subdir/file.txt'); + }); + it('accepts path traversal within base', () => { + expect(safePath('subsubdir/../file.txt', { base })).toBe('data/subdir/file.txt'); + }); + it('rejects path with subdir if noSubDir is true', () => { + expect(safePath('subsubdir/file.txt', { base, noSubDir: true })).toBe(null); + }); }); \ No newline at end of file diff --git a/src/lib/server/database/loadData.ts b/src/lib/server/database/loadData.ts index 4aa66ae..ce4e92d 100644 --- a/src/lib/server/database/loadData.ts +++ b/src/lib/server/database/loadData.ts @@ -2,6 +2,7 @@ import fs from 'fs'; import path from 'path'; import { globalMemoryStorage, songData, songNameCache } from '$lib/server/cache.js'; import { getDirectoryHash } from '../dirHash'; +import { safePath } from '../safePath'; const dataPath = './data/song/'; @@ -26,7 +27,12 @@ export async function loadData() { songNameCache.flushAll(); for (const songID of songList) { try { - const fileContentString = fs.readFileSync(path.join(dataPath, songID + '.json')).toString(); + const normPath = safePath(songID + '.json', { base: dataPath }); + if (!normPath) { + console.error(`[load-song-data] Invalid path for song ID ${songID}`); + continue; + } + const fileContentString = fs.readFileSync(normPath).toString(); const data = JSON.parse(fileContentString); songData.set(songID, data); const metadata: MusicMetadata = data; diff --git a/src/lib/server/safePath.ts b/src/lib/server/safePath.ts new file mode 100644 index 0000000..b7bb4e0 --- /dev/null +++ b/src/lib/server/safePath.ts @@ -0,0 +1,24 @@ +import path from 'path'; + +export function safePath(pathIn: string, options: { base: string, noSubDir?: boolean }): string | null { + const base = options.base.endsWith('/') ? options.base : options.base + '/'; + if (!pathIn.startsWith("./")) { + pathIn = "./" + pathIn; + } + pathIn = path.join(base, pathIn); + const normBase = path.normalize(base); + const normPath = path.normalize(pathIn); + console.log(normBase); + console.log(normPath); + + if (normPath !== normBase && normPath.startsWith(normBase)) { + if (options.noSubDir) { + let rel = path.relative(normBase, normPath); + if (rel.indexOf(path.sep) !== -1) { + return null; + } + } + return normPath; + } + return null; +} \ No newline at end of file diff --git a/src/routes/api/database/song/[id]/+server.ts b/src/routes/api/database/song/[id]/+server.ts index ebc8bde..5d90e70 100644 --- a/src/routes/api/database/song/[id]/+server.ts +++ b/src/routes/api/database/song/[id]/+server.ts @@ -1,10 +1,16 @@ +import { safePath } from '$lib/server/safePath'; import { getCurrentFormattedDateTime } from '$lib/songUpdateTime'; import { json, error } from '@sveltejs/kit'; import fs from 'fs'; import path from 'path'; export async function GET({ params }) { - const filePath = path.join('./data/song', `${params.id}.json`); + const filePath = safePath(`${params.id}.json`, { base: './data/song' }); + if (!filePath) { + return error(404, { + message: "No correspoding song." + }); + } let data; try { data = fs.readFileSync(filePath); } catch (e) { return error(404, { diff --git a/src/routes/database/edit/[id]/+page.server.ts b/src/routes/database/edit/[id]/+page.server.ts index c87a1a0..247677e 100644 --- a/src/routes/database/edit/[id]/+page.server.ts +++ b/src/routes/database/edit/[id]/+page.server.ts @@ -1,10 +1,16 @@ /** @type {import('./$types').PageLoad} */ +import { safePath } from '$lib/server/safePath'; import fs from 'fs'; import path from 'path'; export function load({ params }) { - const filePath = path.join('./data/song', `${params.id}.json`); + const filePath = safePath(`${params.id}.json`, { base: './data/song' }); + if (!filePath) { + return { + songData: null + }; + } try { const dataBuffer = fs.readFileSync(filePath); const data = JSON.parse(dataBuffer.toString());