From 89974f34e3f3c5c85d21b3933d087ad1f1c00711 Mon Sep 17 00:00:00 2001 From: ttsdj Date: Thu, 10 Sep 2026 13:39:18 +0800 Subject: [PATCH] fix(cache): keep download writes inside the downloads directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /_web/cache/save-to-downloads joined the caller-supplied `filename` straight into the downloads directory: const filePath = path.join(downloadsDir, filename) `path.join` collapses `..`, so `{"filename":"../escaped.txt"}` resolved one level above the downloads directory and the route wrote attacker-chosen bytes there, returning `{ success: true }`. Any client that can reach the route could therefore write outside the downloads directory as the app user. The route is registered by the shared web routes on both CLI Web and the Electron internal API, so the fix covers both. Its own siblings already constrain this input — merge.ts and the CLI upload route both run the name through `path.basename` — so the missing guard here was an oversight rather than a documented allowance. Reject anything that is not a plain file name, and join only the validated basename. Names the app itself sends stay unchanged: every caller (conversation export, SQL export, screen capture) already passes a flat file name. --- .../http-routes/src/routes/web/cache.test.ts | 47 +++++++++++++++++++ packages/http-routes/src/routes/web/cache.ts | 10 +++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/http-routes/src/routes/web/cache.test.ts b/packages/http-routes/src/routes/web/cache.test.ts index eb936b0c..e61a1ee2 100644 --- a/packages/http-routes/src/routes/web/cache.test.ts +++ b/packages/http-routes/src/routes/web/cache.test.ts @@ -297,3 +297,50 @@ describe('registerCacheRoutes data directory routes', () => { await app.close() }) }) + +describe('registerCacheRoutes save-to-downloads', () => { + it('writes a plain file name inside the downloads directory', async () => { + const app = Fastify() + registerCacheRoutes(app, { pathProvider: createPathProvider() }) + await app.ready() + + const response = await app.inject({ + method: 'POST', + url: '/_web/cache/save-to-downloads', + payload: { + filename: 'result.csv', + dataUrl: `data:text/csv;charset=utf-8,${encodeURIComponent('a,b\n1,2')}`, + }, + }) + + assert.equal(response.statusCode, 200) + assert.deepEqual(response.json(), { + success: true, + filePath: path.join(testSystemDir, 'downloads', 'result.csv'), + }) + + await app.close() + }) + + it('refuses a file name that would escape the downloads directory', async () => { + const app = Fastify() + registerCacheRoutes(app, { pathProvider: createPathProvider() }) + await app.ready() + + const escapedPath = path.join(testSystemDir, 'escaped.txt') + const response = await app.inject({ + method: 'POST', + url: '/_web/cache/save-to-downloads', + payload: { + filename: '../escaped.txt', + dataUrl: `data:text/plain;charset=utf-8,${encodeURIComponent('attacker controlled')}`, + }, + }) + + assert.equal(response.statusCode, 200) + assert.deepEqual(response.json(), { success: false, error: 'Invalid filename' }) + assert.equal(fs.existsSync(escapedPath), false) + + await app.close() + }) +}) diff --git a/packages/http-routes/src/routes/web/cache.ts b/packages/http-routes/src/routes/web/cache.ts index b20ae609..e07665ad 100644 --- a/packages/http-routes/src/routes/web/cache.ts +++ b/packages/http-routes/src/routes/web/cache.ts @@ -267,7 +267,15 @@ export function registerCacheRoutes(server: FastifyInstance, ctx: CacheRouteCont fs.mkdirSync(downloadsDir, { recursive: true }) } - const filePath = path.join(downloadsDir, filename) + // Confine the write to the downloads directory: browser callers pick this + // name, and `path.join` collapses `..`, so anything but a plain file name + // would otherwise resolve outside `downloadsDir`. + const safeName = path.basename(filename) + if (safeName !== filename || safeName === '.' || safeName === '..') { + return { success: false, error: 'Invalid filename' } + } + + const filePath = path.join(downloadsDir, safeName) fs.writeFileSync(filePath, buffer) return { success: true, filePath }