diff --git a/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch b/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch deleted file mode 100644 index d7ee2e285e..0000000000 --- a/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch +++ /dev/null @@ -1,10 +0,0 @@ -diff --git a/methods/inflater.js b/methods/inflater.js -index 8769e66e82b25541aba80b1ac6429199c9a8179f..1d4402402f0e1aaf64062c1f004c3d6e6fe93e76 100644 ---- a/methods/inflater.js -+++ b/methods/inflater.js -@@ -1,4 +1,4 @@ --const version = +(process.versions ? process.versions.node : "").split(".")[0] || 0; -+const version = +(process?.versions?.node ?? "").split(".")[0] || 0; - - module.exports = function (/*Buffer*/ inbuf, /*number*/ expectedLength) { - var zlib = require("zlib"); diff --git a/package.json b/package.json index e6c581b48c..5086204989 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,6 @@ "glob-hasher-linux-arm64-gnu": "1.4.2" }, "resolutions": { - "adm-zip": "patch:adm-zip@npm%3A0.5.16#~/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch", "minimatch@3.1.2": "3.1.5", "serialize-javascript": "^7.0.5", "socks": ">=2.8.3", diff --git a/packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts b/packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts index f52dea15b0..15ea6c4628 100644 --- a/packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts +++ b/packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts @@ -1,5 +1,6 @@ import { send } from '@actual-app/core/platform/client/connection'; import type { RemoteFile } from '@actual-app/core/server/cloud-storage'; +import { getUnsafeZipMeta } from '@actual-app/core/shared/errors'; import type { Budget } from '@actual-app/core/types/budget'; import type { File } from '@actual-app/core/types/file'; import type { Handlers } from '@actual-app/core/types/handlers'; @@ -12,7 +13,7 @@ import { closeModal, pushModal } from '#modals/modalsSlice'; import { loadGlobalPrefs, loadPrefs } from '#prefs/prefsSlice'; import { createAppAsyncThunk } from '#redux'; import { signOut } from '#users/usersSlice'; -import { getDownloadError, getSyncError } from '#util/error'; +import { getDownloadError, getSyncError, getUnsafeZipError } from '#util/error'; const sliceName = 'budgetfiles'; @@ -243,9 +244,10 @@ type ImportBudgetPayload = { export const importBudget = createAppAsyncThunk( `${sliceName}/importBudget`, async ({ filepath, type }: ImportBudgetPayload, { dispatch }) => { - const { error } = await send('import-budget', { filepath, type }); + const { error, meta } = await send('import-budget', { filepath, type }); if (error) { - throw new Error(error); + const zipMeta = getUnsafeZipMeta(meta); + throw new Error(zipMeta ? getUnsafeZipError(zipMeta) : error); } dispatch(closeModal()); diff --git a/packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx b/packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx index d838738a80..91724bd46a 100644 --- a/packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx +++ b/packages/desktop-client/src/components/modals/manager/ImportActualModal.tsx @@ -39,6 +39,10 @@ export function ImportActualModal() { return t('This archive is not a valid Actual export file.'); case 'invalid-metadata-file': return t('The metadata file in the given archive is corrupted.'); + case 'zip-too-large': + return t( + 'This file is too large to import, sorry! Visit https://actualbudget.org/contact/ for support.', + ); default: return t( 'An unknown error occurred while importing. Please report this as a new issue on GitHub.', diff --git a/packages/desktop-client/src/components/settings/Export.tsx b/packages/desktop-client/src/components/settings/Export.tsx index 617a48f2fe..efc39c800f 100644 --- a/packages/desktop-client/src/components/settings/Export.tsx +++ b/packages/desktop-client/src/components/settings/Export.tsx @@ -16,12 +16,14 @@ export function ExportBudget() { const { t } = useTranslation(); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(null); + const [warnings, setWarnings] = useState([]); const [budgetName] = useMetadataPref('budgetName'); const [encryptKeyId] = useMetadataPref('encryptKeyId'); async function onExport() { setIsLoading(true); setError(null); + setWarnings([]); const response = await send('export-budget'); @@ -33,6 +35,7 @@ export function ExportBudget() { } if (response.data) { + setWarnings(response.warnings ?? []); void window.Actual.saveFile( response.data, `${format(new Date(), 'yyyy-MM-dd')}-${budgetName}.zip`, @@ -56,6 +59,22 @@ export function ExportBudget() { )} )} + {warnings.includes('exceeds-import-size-limit') && ( + + + This export is larger than Actual can safely re-import. You may + not be able to restore this backup. + + + )} + {warnings.includes('may-exceed-available-memory') && ( + + + This export is larger than the memory available on this device. + Restoring it here may fail. + + + )} } > diff --git a/packages/desktop-client/src/util/error.ts b/packages/desktop-client/src/util/error.ts index 53afcd7fe7..ed1bafa0dd 100644 --- a/packages/desktop-client/src/util/error.ts +++ b/packages/desktop-client/src/util/error.ts @@ -1,3 +1,5 @@ +import { getUnsafeZipMeta, toMB } from '@actual-app/core/shared/errors'; +import type { UnsafeZipMeta } from '@actual-app/core/shared/errors'; import { t } from 'i18next'; type ErrorWithMeta = { @@ -5,6 +7,45 @@ type ErrorWithMeta = { meta?: unknown; }; +export function getUnsafeZipError({ + zipReason, + entryName, + maxSize, +}: UnsafeZipMeta): string { + const maxSizeMB = toMB(maxSize); + switch (zipReason) { + case 'archive-size': + return t( + 'This file is larger than the maximum supported size of {{maxSizeMB}} MB, sorry! Visit https://actualbudget.org/contact/ for support.', + { maxSizeMB }, + ); + case 'entry-size': + return t( + 'The file "{{entryName}}" in this archive is larger than the maximum supported size of {{maxSizeMB}} MB, sorry! Visit https://actualbudget.org/contact/ for support.', + { entryName, maxSizeMB }, + ); + case 'total-size': + return t( + 'The uncompressed contents of this archive are larger than the maximum supported size of {{maxSizeMB}} MB, sorry! Visit https://actualbudget.org/contact/ for support.', + { maxSizeMB }, + ); + case 'unsafe-entry-name': + return t( + 'This archive contains an entry with an unsafe file name: "{{entryName}}".', + { entryName }, + ); + case 'duplicate-entry': + return t( + 'This archive contains more than one entry named "{{entryName}}".', + { entryName }, + ); + default: + return t( + 'This file could not be imported, sorry! Visit https://actualbudget.org/contact/ for support.', + ); + } +} + export function getUploadError({ reason, meta }: ErrorWithMeta) { switch (reason) { case 'unauthorized': @@ -88,6 +129,15 @@ export function getDownloadError({ return t( 'Downloaded file is invalid, sorry! Visit https://actualbudget.org/contact/ for support.', ); + case 'zip-too-large': { + const zipMeta = getUnsafeZipMeta(meta); + if (zipMeta) { + return getUnsafeZipError(zipMeta); + } + return t( + 'This file is too large to import, sorry! Visit https://actualbudget.org/contact/ for support.', + ); + } case 'decrypt-failure': return t( 'Unable to decrypt file {{fileName}}. To change your key, first download this file with the proper password.', diff --git a/packages/loot-core/package.json b/packages/loot-core/package.json index d6d08f6cd0..2a94d6aa9b 100644 --- a/packages/loot-core/package.json +++ b/packages/loot-core/package.json @@ -51,6 +51,11 @@ "api": "./src/platform/server/fs/path-join.api.ts", "default": "./src/platform/server/fs/path-join.ts" }, + "#platform/server/memory": { + "electron": "./src/platform/server/memory/index.electron.ts", + "api": "./src/platform/server/memory/index.api.ts", + "default": "./src/platform/server/memory/index.ts" + }, "#platform/server/sqlite": { "electron": "./src/platform/server/sqlite/index.electron.ts", "api": "./src/platform/server/sqlite/index.api.ts", @@ -195,12 +200,12 @@ "@rschedule/core": "^1.5.0", "@rschedule/standard-date-adapter": "^1.5.0", "absurd-sql": "0.0.54", - "adm-zip": "^0.5.17", "better-sqlite3": "^12.10.0", "csv-parse": "^6.2.1", "csv-stringify": "^6.7.0", "date-fns": "^4.4.0", "es-toolkit": "^1.47.1", + "fflate": "^0.8.3", "handlebars": "^4.7.9", "hyperformula": "^3.3.0", "lru-cache": "^11.5.1", @@ -211,7 +216,6 @@ "devDependencies": { "@actual-app/crdt": "workspace:*", "@actual-app/vite-plugin-peggy": "workspace:*", - "@types/adm-zip": "^0.5.8", "@types/better-sqlite3": "^7.6.13", "@types/jlongster__sql.js": "npm:@types/sql.js@latest", "@types/node": "^22.19.21", diff --git a/packages/loot-core/src/platform/server/memory/index.api.ts b/packages/loot-core/src/platform/server/memory/index.api.ts new file mode 100644 index 0000000000..e3909ec40e --- /dev/null +++ b/packages/loot-core/src/platform/server/memory/index.api.ts @@ -0,0 +1,6 @@ +import * as os from 'os'; + +import type * as T from './index'; + +export const getAvailableMemory: typeof T.getAvailableMemory = () => + os.freemem(); diff --git a/packages/loot-core/src/platform/server/memory/index.electron.ts b/packages/loot-core/src/platform/server/memory/index.electron.ts new file mode 100644 index 0000000000..9c98dddf3d --- /dev/null +++ b/packages/loot-core/src/platform/server/memory/index.electron.ts @@ -0,0 +1,2 @@ +// oxlint-disable-next-line no-restricted-imports +export * from './index.api'; diff --git a/packages/loot-core/src/platform/server/memory/index.ts b/packages/loot-core/src/platform/server/memory/index.ts new file mode 100644 index 0000000000..d49972fc5b --- /dev/null +++ b/packages/loot-core/src/platform/server/memory/index.ts @@ -0,0 +1,4 @@ +// No OS-level memory API is available in a browser context. +export function getAvailableMemory(): number | null { + return null; +} diff --git a/packages/loot-core/src/server/budgetfiles/app.ts b/packages/loot-core/src/server/budgetfiles/app.ts index da6fbce9b9..f5e50384fa 100644 --- a/packages/loot-core/src/server/budgetfiles/app.ts +++ b/packages/loot-core/src/server/budgetfiles/app.ts @@ -481,7 +481,7 @@ async function importBudget({ * to derive the budget name for some import types. */ filename?: string; -}): Promise<{ error?: string; id?: string }> { +}): Promise<{ error?: string; meta?: unknown; id?: string }> { try { let contents: Buffer; let name: string; @@ -514,8 +514,10 @@ async function importBudget({ async function exportBudget() { try { + const exported = await cloudStorage.exportBuffer(); return { - data: await cloudStorage.exportBuffer(), + data: exported?.data ?? null, + warnings: exported?.warnings ?? [], }; } catch (err) { err.message = 'Error exporting budget: ' + err.message; diff --git a/packages/loot-core/src/server/budgetfiles/backups.ts b/packages/loot-core/src/server/budgetfiles/backups.ts index f0da366ee0..5a0d9366e4 100644 --- a/packages/loot-core/src/server/budgetfiles/backups.ts +++ b/packages/loot-core/src/server/budgetfiles/backups.ts @@ -1,6 +1,5 @@ import type { Database } from '@jlongster/sql.js'; // @ts-strict-ignore -import AdmZip from 'adm-zip'; import * as dateFns from 'date-fns'; import * as connection from '#platform/server/connection'; @@ -9,6 +8,7 @@ import { logger } from '#platform/server/log'; import * as sqlite from '#platform/server/sqlite'; import * as cloudStorage from '#server/cloud-storage'; import * as prefs from '#server/prefs'; +import { safeUnzip, safeZip } from '#server/util/zip'; import * as monthUtils from '#shared/months'; // A special backup that represents the latest version of the db that @@ -137,10 +137,16 @@ export async function makeBackup(id: string) { sqlite.runQuery(db, 'DELETE FROM messages_crdt'); sqlite.runQuery(db, 'DELETE FROM messages_clock'); // Zip up the cleaned db and metadata into a single backup file - const zip = new AdmZip(); - zip.addLocalFile(tempDbPath, '', 'db.sqlite'); - zip.addLocalFile(fs.join(budgetDir, 'metadata.json')); - zip.writeZip(backupPath); + const dbContent = await fs.readFile(tempDbPath, 'binary'); + const metaContent = await fs.readFile( + fs.join(budgetDir, 'metadata.json'), + 'binary', + ); + const zipped = safeZip({ + 'db.sqlite': dbContent, + 'metadata.json': metaContent, + }); + await fs.writeFile(backupPath, zipped); } finally { if (db) { sqlite.closeDatabase(db); @@ -224,9 +230,27 @@ export async function loadBackup(id: string, backupId: string) { prefs.unloadPrefs(); - const zip = new AdmZip(fs.join(budgetDir, 'backups', backupId)); - zip.extractEntryTo('db.sqlite', budgetDir, false, true); - zip.extractEntryTo('metadata.json', budgetDir, false, true); + const zipContent = await fs.readFile( + fs.join(budgetDir, 'backups', backupId), + 'binary', + ); + + let entries: Record; + try { + entries = safeUnzip(zipContent); + } catch (e) { + logger.log(e); + throw new Error('Error reading backup zip file'); + } + if (!entries['db.sqlite'] || !entries['metadata.json']) { + throw new Error('Backup zip file is missing db.sqlite or metadata.json'); + } + + await fs.writeFile(fs.join(budgetDir, 'db.sqlite'), entries['db.sqlite']); + await fs.writeFile( + fs.join(budgetDir, 'metadata.json'), + entries['metadata.json'], + ); } } diff --git a/packages/loot-core/src/server/cloud-storage.ts b/packages/loot-core/src/server/cloud-storage.ts index 5633e03c37..c79a202db3 100644 --- a/packages/loot-core/src/server/cloud-storage.ts +++ b/packages/loot-core/src/server/cloud-storage.ts @@ -1,11 +1,11 @@ // @ts-strict-ignore -import AdmZip from 'adm-zip'; import { v4 as uuidv4 } from 'uuid'; import * as asyncStorage from '#platform/server/asyncStorage'; import { fetch } from '#platform/server/fetch'; import * as fs from '#platform/server/fs'; import { logger } from '#platform/server/log'; +import * as memory from '#platform/server/memory'; import * as sqlite from '#platform/server/sqlite'; import * as monthUtils from '#shared/months'; @@ -20,6 +20,12 @@ import { runMutator } from './mutators'; import { post } from './post'; import * as prefs from './prefs'; import { getServer } from './server-config'; +import { + exceedsSafeUnzipLimits, + safeUnzip, + safeZip, + UnsafeZipError, +} from './util/zip'; const UPLOAD_FREQUENCY_IN_DAYS = 7; @@ -145,14 +151,11 @@ export async function exportBuffer() { const budgetDir = fs.getBudgetDir(id); - // create zip - const zipped = new AdmZip(); - // We run this in a mutator even though its not mutating anything // because we are reading the sqlite file from disk. We want to make // sure that we get a valid snapshot of it so we want this to be // serialized with all other mutations. - await runMutator(async () => { + const { zipped, entries } = await runMutator(async () => { const rawDbContent = await fs.readFile( fs.join(budgetDir, 'db.sqlite'), 'binary', @@ -183,30 +186,66 @@ export async function exportBuffer() { meta.resetClock = true; const metaContent = Buffer.from(JSON.stringify(meta), 'utf8'); - zipped.addFile('db.sqlite', Buffer.from(dbContent)); - zipped.addFile('metadata.json', metaContent); + const entries = { + 'db.sqlite': Buffer.from(dbContent), + 'metadata.json': metaContent, + }; + + return { zipped: safeZip(entries), entries }; }); - return Buffer.from(zipped.toBuffer()); + const warnings: string[] = []; + if (exceedsSafeUnzipLimits(zipped, entries)) { + warnings.push('exceeds-import-size-limit'); + } + + const availableMemory = memory.getAvailableMemory(); + if ( + availableMemory != null && + entries['db.sqlite'].length > availableMemory + ) { + warnings.push('may-exceed-available-memory'); + } + + return { data: Buffer.from(zipped), warnings }; } export async function importBuffer(fileData, buffer) { - let zipped, entries; + let entries; try { - zipped = new AdmZip(buffer); - entries = zipped.getEntries(); - } catch { + entries = safeUnzip(buffer); + } catch (e) { + if (e instanceof UnsafeZipError) { + throw FileDownloadError('zip-too-large', e.meta); + } throw FileDownloadError('not-zip-file'); } - const dbEntry = entries.find(e => e.entryName.includes('db.sqlite')); - const metaEntry = entries.find(e => e.entryName.includes('metadata.json')); + const entryNames = Object.keys(entries); + const dbDirs = entryNames + .filter(name => name === 'db.sqlite' || name.endsWith('/db.sqlite')) + .map(name => name.slice(0, -'db.sqlite'.length)); + const metaDirs = entryNames + .filter(name => name === 'metadata.json' || name.endsWith('/metadata.json')) + .map(name => name.slice(0, -'metadata.json'.length)); - if (!dbEntry || !metaEntry) { + // Both files must come from the same directory: prefer the archive root, + // otherwise there must be exactly one directory containing both. + const sharedDirs = dbDirs.filter(dir => metaDirs.includes(dir)); + const dir = sharedDirs.includes('') + ? '' + : sharedDirs.length === 1 + ? sharedDirs[0] + : null; + + if (dir == null) { throw FileDownloadError('invalid-zip-file'); } - const dbContent = zipped.readFile(dbEntry); - const metaContent = zipped.readFile(metaEntry); + const entryName = dir + 'db.sqlite'; + const metaEntryName = dir + 'metadata.json'; + + const dbContent = Buffer.from(entries[entryName]); + const metaContent = Buffer.from(entries[metaEntryName]); let meta; try { @@ -254,10 +293,11 @@ export async function upload() { throw FileUploadError('unauthorized'); } - const zipContent = await exportBuffer(); - if (zipContent == null) { + const exported = await exportBuffer(); + if (exported == null) { return; } + const zipContent = exported.data; const { id, diff --git a/packages/loot-core/src/server/errors.ts b/packages/loot-core/src/server/errors.ts index e304f401e5..86ae93a37f 100644 --- a/packages/loot-core/src/server/errors.ts +++ b/packages/loot-core/src/server/errors.ts @@ -1,3 +1,5 @@ +import type { UnsafeZipMeta } from '#shared/errors'; + // TODO: normalize error types export class PostError extends Error { meta: { meta: string } | undefined; @@ -111,7 +113,7 @@ export function FileDownloadError( isMissingKey?: boolean; name?: string; id?: string; - }, + } & Partial, ) { return { type: 'FileDownloadError', reason, meta }; } diff --git a/packages/loot-core/src/server/importers/actual.ts b/packages/loot-core/src/server/importers/actual.ts index 7a5070d116..b8fca76bdb 100644 --- a/packages/loot-core/src/server/importers/actual.ts +++ b/packages/loot-core/src/server/importers/actual.ts @@ -19,7 +19,7 @@ export async function importActual(_filepath: string, buffer: Buffer) { )); } catch (e) { if (e.type === 'FileDownloadError') { - return { error: e.reason }; + return { error: e.reason, meta: e.meta }; } throw e; } diff --git a/packages/loot-core/src/server/importers/ynab4.ts b/packages/loot-core/src/server/importers/ynab4.ts index 021f5b6b87..f8dbdf2788 100644 --- a/packages/loot-core/src/server/importers/ynab4.ts +++ b/packages/loot-core/src/server/importers/ynab4.ts @@ -1,9 +1,9 @@ // @ts-strict-ignore -import AdmZip from 'adm-zip'; import { v4 as uuidv4 } from 'uuid'; import { logger } from '#platform/server/log'; import { send } from '#server/main-app'; +import { safeUnzip } from '#server/util/zip'; import * as monthUtils from '#shared/months'; import { amountToInteger, groupBy, sortByKey } from '#shared/util'; @@ -343,10 +343,13 @@ function estimateRecentness(str: string) { }, 0); } -function findLatestDevice(zipped: AdmZip, entries: AdmZip.IZipEntry[]): string { +function findLatestDevice( + zipped: Record, + entries: string[], +): string { let devices = entries .map(entry => { - const contents = zipped.readFile(entry).toString('utf8'); + const contents = Buffer.from(zipped[entry]).toString('utf8'); let data; try { @@ -411,8 +414,8 @@ export function getBudgetName(filepath) { return m[1]; } -function getFile(entries: AdmZip.IZipEntry[], path: string) { - const files = entries.filter(e => e.entryName === path); +function getFile(entries: string[], path: string) { + const files = entries.filter(e => e === path); if (files.length === 0) { throw new Error('Could not find file: ' + path); } @@ -432,28 +435,36 @@ function join(...paths: string[]): string { } export function parseFile(buffer: Buffer): YNAB4.YFull { - const zipped = new AdmZip(buffer); - const entries = zipped.getEntries(); + let zipped: Record; + try { + zipped = safeUnzip(buffer); + } catch (e) { + logger.log(e); + throw new Error('Error reading zip file'); + } + const entries = Object.keys(zipped); let root = ''; - const dirMatch = entries[0].entryName.match(/([^/]*\.ynab4)/); + const dirMatch = entries[0].match(/([^/]*\.ynab4)/); if (dirMatch) { root = dirMatch[1] + '/'; } - const metaStr = zipped.readFile(getFile(entries, root + 'Budget.ymeta')); + const metaStr = Buffer.from(zipped[getFile(entries, root + 'Budget.ymeta')]); const meta = JSON.parse(metaStr.toString('utf8')); const budgetPath = join(root, meta.relativeDataFolderName); const deviceFiles = entries.filter(e => - e.entryName.startsWith(join(budgetPath, 'devices')), + e.startsWith(join(budgetPath, 'devices')), ); const deviceGUID = findLatestDevice(zipped, deviceFiles); const yfullPath = join(budgetPath, deviceGUID, 'Budget.yfull'); let contents; try { - contents = zipped.readFile(getFile(entries, yfullPath)).toString('utf8'); + contents = Buffer.from(zipped[getFile(entries, yfullPath)]).toString( + 'utf8', + ); } catch (e) { logger.log(e); throw new Error('Error reading Budget.yfull file'); diff --git a/packages/loot-core/src/server/util/zip.test.ts b/packages/loot-core/src/server/util/zip.test.ts new file mode 100644 index 0000000000..13a07a2922 --- /dev/null +++ b/packages/loot-core/src/server/util/zip.test.ts @@ -0,0 +1,149 @@ +import { zipSync } from 'fflate'; +import { afterAll, beforeAll, describe, expect, test } from 'vitest'; + +import { + exceedsSafeUnzipLimits, + safeUnzip, + safeZip, + UnsafeZipError, +} from './zip'; + +// fflate stamps each entry with the current date in DOS format, which +// rejects the fake pre-1980 clock the test setup freezes `Date.now` to. +const testGlobal = global as unknown as { + restoreDateNow: () => void; + restoreFakeDateNow: () => void; +}; +beforeAll(() => testGlobal.restoreDateNow()); +afterAll(() => testGlobal.restoreFakeDateNow()); + +describe('safeZip / safeUnzip round trip', () => { + test('unzips exactly what was zipped', () => { + const zipped = safeZip({ + 'db.sqlite': Buffer.from('sqlite-content'), + 'metadata.json': Buffer.from('{"id":"abc"}'), + }); + + const entries = safeUnzip(zipped); + + expect(Buffer.from(entries['db.sqlite']).toString()).toBe('sqlite-content'); + expect(Buffer.from(entries['metadata.json']).toString()).toBe( + '{"id":"abc"}', + ); + }); + + test('round trips a nested entry name', () => { + const zipped = safeZip({ + 'budget/db.sqlite': Buffer.from('nested'), + }); + + const entries = safeUnzip(zipped); + + expect(Buffer.from(entries['budget/db.sqlite']).toString()).toBe('nested'); + }); +}); + +describe('unsafe entry names', () => { + test.each([ + ['path traversal', '../etc/passwd'], + ['path traversal in a nested segment', 'foo/../../etc/passwd'], + ['absolute path', '/etc/passwd'], + ['backslash', 'foo\\bar'], + ['windows drive letter', 'C:\\evil.txt'], + ['null byte', 'foo\0bar'], + ])('safeZip rejects %s', (_name, entryName) => { + expect(() => safeZip({ [entryName]: Buffer.from('x') })).toThrow( + UnsafeZipError, + ); + }); + + test.each([ + ['path traversal', '../etc/passwd'], + ['absolute path', '/etc/passwd'], + ['backslash', 'foo\\bar'], + ['windows drive letter', 'C:\\evil.txt'], + ])('safeUnzip rejects %s', (_name, entryName) => { + // Build the archive with fflate directly since safeZip would already + // reject these names before we get a chance to unzip them. + const zipped = zipSync({ [entryName]: Buffer.from('x') }); + + expect(() => safeUnzip(zipped)).toThrow(UnsafeZipError); + }); +}); + +describe('duplicate entries', () => { + test('safeUnzip rejects case-insensitive duplicate names', () => { + const zipped = zipSync({ + 'db.sqlite': Buffer.from('a'), + 'DB.SQLITE': Buffer.from('b'), + }); + + expect(() => safeUnzip(zipped)).toThrow(UnsafeZipError); + }); +}); + +describe('size limits', () => { + test('safeUnzip rejects an archive over maxArchiveSize', () => { + const zipped = safeZip({ 'db.sqlite': Buffer.from('x'.repeat(1000)) }); + + expect(() => safeUnzip(zipped, { maxArchiveSize: 10 })).toThrow( + UnsafeZipError, + ); + }); + + test('safeUnzip rejects a single entry over maxEntrySize', () => { + const zipped = safeZip({ 'db.sqlite': Buffer.from('x'.repeat(1000)) }); + + expect(() => + safeUnzip(zipped, { maxArchiveSize: 10_000, maxEntrySize: 10 }), + ).toThrow(UnsafeZipError); + }); + + test('safeUnzip rejects when total uncompressed size exceeds the cap', () => { + const zipped = safeZip({ + a: Buffer.from('x'.repeat(100)), + b: Buffer.from('x'.repeat(100)), + }); + + expect(() => + safeUnzip(zipped, { + maxArchiveSize: 10_000, + maxEntrySize: 10_000, + maxTotalUncompressedSize: 150, + }), + ).toThrow(UnsafeZipError); + }); + + test('safeUnzip accepts an archive within all limits', () => { + const zipped = safeZip({ 'db.sqlite': Buffer.from('small') }); + + expect(() => safeUnzip(zipped)).not.toThrow(); + }); +}); + +describe('exceedsSafeUnzipLimits', () => { + test('reports false for a small archive', () => { + const entries = { 'db.sqlite': Buffer.from('small') }; + const zipped = safeZip(entries); + + expect(exceedsSafeUnzipLimits(zipped, entries)).toBe(false); + }); + + test('reports true when an entry is larger than safeUnzip would allow', () => { + // Simulate an oversized entry without actually allocating 500MB+. + const bigEntry = { length: 600 * 1024 * 1024 } as unknown as Uint8Array; + const smallArchive = new Uint8Array(10); + + expect( + exceedsSafeUnzipLimits(smallArchive, { 'db.sqlite': bigEntry }), + ).toBe(true); + }); + + test('reports true when the archive itself is larger than safeUnzip would allow', () => { + const bigArchive = { + length: 2 * 1024 * 1024 * 1024, + } as unknown as Uint8Array; + + expect(exceedsSafeUnzipLimits(bigArchive, {})).toBe(true); + }); +}); diff --git a/packages/loot-core/src/server/util/zip.ts b/packages/loot-core/src/server/util/zip.ts new file mode 100644 index 0000000000..c3a9458c83 --- /dev/null +++ b/packages/loot-core/src/server/util/zip.ts @@ -0,0 +1,123 @@ +import { unzipSync, zipSync } from 'fflate'; +import type { Unzipped } from 'fflate'; + +import type { UnsafeZipMeta } from '#shared/errors'; + +// fflate does no validation itself: guard against zip-slip, decompression +// bombs, and duplicate entries. + +const MAX_ZIP_SIZE = 500 * 1024 * 1024; // 500MB; also doubles as a memory-safety cap + +export class UnsafeZipError extends Error { + readonly meta: UnsafeZipMeta; + + constructor(message: string, meta: UnsafeZipMeta) { + super(message); + this.meta = meta; + } +} + +function assertSafeEntryName(name: string) { + const isTraversal = name.split('/').some(segment => segment === '..'); + + if ( + name.includes('\0') || + name.includes('\\') || + /^[a-zA-Z]:/.test(name) || + name.startsWith('/') || + isTraversal + ) { + throw new UnsafeZipError(`Unsafe zip entry name: ${name}`, { + zipReason: 'unsafe-entry-name', + entryName: name, + }); + } +} + +type SafeUnzipOptions = { + maxArchiveSize?: number; + maxEntrySize?: number; + maxTotalUncompressedSize?: number; +}; + +export function safeUnzip( + data: Uint8Array, + { + maxArchiveSize = MAX_ZIP_SIZE, + maxEntrySize = MAX_ZIP_SIZE, + maxTotalUncompressedSize = MAX_ZIP_SIZE, + }: SafeUnzipOptions = {}, +): Unzipped { + if (data.length > maxArchiveSize) { + throw new UnsafeZipError( + `Zip archive exceeds maximum size of ${maxArchiveSize} bytes`, + { zipReason: 'archive-size', maxSize: maxArchiveSize }, + ); + } + + const seen = new Set(); + + let totalUncompressedSize = 0; + + return unzipSync(data, { + filter(file) { + assertSafeEntryName(file.name); + + if (file.originalSize > maxEntrySize) { + throw new UnsafeZipError( + `Zip entry "${file.name}" exceeds maximum size of ${maxEntrySize} bytes`, + { + zipReason: 'entry-size', + entryName: file.name, + maxSize: maxEntrySize, + }, + ); + } + + totalUncompressedSize += file.originalSize; + if (totalUncompressedSize > maxTotalUncompressedSize) { + throw new UnsafeZipError( + `Zip archive's total uncompressed size exceeds maximum of ${maxTotalUncompressedSize} bytes`, + { zipReason: 'total-size', maxSize: maxTotalUncompressedSize }, + ); + } + + const normalized = file.name.toLowerCase(); + if (seen.has(normalized)) { + throw new UnsafeZipError( + `Zip archive contains a duplicate entry: ${file.name}`, + { zipReason: 'duplicate-entry', entryName: file.name }, + ); + } + seen.add(normalized); + + return true; + }, + }); +} + +export function safeZip(files: Record): Uint8Array { + for (const name of Object.keys(files)) { + assertSafeEntryName(name); + } + return zipSync(files); +} + +export function exceedsSafeUnzipLimits( + archive: Uint8Array, + entries: Record, +): boolean { + if (archive.length > MAX_ZIP_SIZE) { + return true; + } + + let totalUncompressedSize = 0; + for (const entry of Object.values(entries)) { + if (entry.length > MAX_ZIP_SIZE) { + return true; + } + totalUncompressedSize += entry.length; + } + + return totalUncompressedSize > MAX_ZIP_SIZE; +} diff --git a/packages/loot-core/src/shared/errors.ts b/packages/loot-core/src/shared/errors.ts index 01d2e826fd..237654831e 100644 --- a/packages/loot-core/src/shared/errors.ts +++ b/packages/loot-core/src/shared/errors.ts @@ -3,6 +3,52 @@ type ErrorWithMeta = { meta?: unknown; }; +export type UnsafeZipMeta = { + zipReason: + | 'unsafe-entry-name' + | 'archive-size' + | 'entry-size' + | 'total-size' + | 'duplicate-entry'; + entryName?: string; + maxSize?: number; +}; + +export function getUnsafeZipMeta(meta?: unknown): UnsafeZipMeta | null { + if ( + meta && + typeof meta === 'object' && + 'zipReason' in meta && + typeof meta.zipReason === 'string' + ) { + return meta as UnsafeZipMeta; + } + return null; +} + +export function toMB(bytes?: number): number | null { + return bytes != null ? Math.round(bytes / (1024 * 1024)) : null; +} + +function getUnsafeZipError(meta: UnsafeZipMeta): string { + const { entryName } = meta; + const maxSizeMB = toMB(meta.maxSize); + switch (meta.zipReason) { + case 'archive-size': + return `This file is larger than the maximum supported size of ${maxSizeMB} MB, sorry! Visit https://actualbudget.org/contact/ for support.`; + case 'entry-size': + return `The file "${entryName}" in this archive is larger than the maximum supported size of ${maxSizeMB} MB, sorry! Visit https://actualbudget.org/contact/ for support.`; + case 'total-size': + return `The uncompressed contents of this archive are larger than the maximum supported size of ${maxSizeMB} MB, sorry! Visit https://actualbudget.org/contact/ for support.`; + case 'unsafe-entry-name': + return `This archive contains an entry with an unsafe file name: "${entryName}".`; + case 'duplicate-entry': + return `This archive contains more than one entry named "${entryName}".`; + default: + return 'This file could not be imported, sorry! Visit https://actualbudget.org/contact/ for support.'; + } +} + // NOTE: These error formatters are consumed by the headless `@actual-app/api` // (see `src/server/api.ts`), which has no i18n. They intentionally return // plain English strings. User-facing, translated equivalents live in the @@ -48,6 +94,13 @@ export function getDownloadError({ case 'invalid-zip-file': case 'invalid-meta-file': return 'Downloaded file is invalid, sorry! Visit https://actualbudget.org/contact/ for support.'; + case 'zip-too-large': { + const zipMeta = getUnsafeZipMeta(meta); + if (zipMeta) { + return getUnsafeZipError(zipMeta); + } + return 'This file is too large to import, sorry! Visit https://actualbudget.org/contact/ for support.'; + } case 'decrypt-failure': return ( 'Unable to decrypt file ' + diff --git a/upcoming-release-notes/replace-adm-zip-with-fflate.md b/upcoming-release-notes/replace-adm-zip-with-fflate.md new file mode 100644 index 0000000000..446801dffe --- /dev/null +++ b/upcoming-release-notes/replace-adm-zip-with-fflate.md @@ -0,0 +1,6 @@ +--- +category: Maintenance +authors: [StephenBrown2] +--- + +Replace adm-zip with fflate and safe wrappers for zip handling. diff --git a/yarn.lock b/yarn.lock index 5599fcc988..e0b1dbf4f3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -107,13 +107,11 @@ __metadata: "@jlongster/sql.js": "npm:^1.6.7" "@rschedule/core": "npm:^1.5.0" "@rschedule/standard-date-adapter": "npm:^1.5.0" - "@types/adm-zip": "npm:^0.5.8" "@types/better-sqlite3": "npm:^7.6.13" "@types/jlongster__sql.js": "npm:@types/sql.js@latest" "@types/node": "npm:^22.19.21" "@typescript/native-preview": "npm:beta" absurd-sql: "npm:0.0.54" - adm-zip: "npm:^0.5.17" assert: "npm:^2.1.0" better-sqlite3: "npm:^12.10.0" browserify-zlib: "npm:^0.2.0" @@ -126,6 +124,7 @@ __metadata: es-toolkit: "npm:^1.47.1" fake-indexeddb: "npm:^6.2.5" fast-check: "npm:^4.8.0" + fflate: "npm:^0.8.3" handlebars: "npm:^4.7.9" hyperformula: "npm:^3.3.0" jest-diff: "npm:^30.4.1" @@ -8654,15 +8653,6 @@ __metadata: languageName: node linkType: hard -"@types/adm-zip@npm:^0.5.8": - version: 0.5.8 - resolution: "@types/adm-zip@npm:0.5.8" - dependencies: - "@types/node": "npm:*" - checksum: 10/0739e5abda05f5c02b84048a02d308f77dd9e2d4e7d6ac7126830a0000b9e962a1f5795efd96d50df289333af9ae3b264adacfad9623dde66c10570ce65029d5 - languageName: node - linkType: hard - "@types/aria-query@npm:^5.0.1": version: 5.0.4 resolution: "@types/aria-query@npm:5.0.4" @@ -10514,20 +10504,6 @@ __metadata: languageName: node linkType: hard -"adm-zip@npm:0.5.16": - version: 0.5.16 - resolution: "adm-zip@npm:0.5.16" - checksum: 10/e167d1b9e60cde37334efda828fa514680af9facbd4183952f36526390e5c7da9a90ca1e6880dfd3aba7b3517f1506c6178e0dc29cd630b26b98c795f97fc599 - languageName: node - linkType: hard - -"adm-zip@patch:adm-zip@npm%3A0.5.16#~/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch": - version: 0.5.16 - resolution: "adm-zip@patch:adm-zip@npm%3A0.5.16#~/.yarn/patches/adm-zip-npm-0.5.16-4556fea098.patch::version=0.5.16&hash=f6b7d0" - checksum: 10/1a8c866a6ad1b6b6870d39468f26369cc6a43027678708cd9d40ff64274bb45b946f570293d0ac5d23a4a2b360e36168f14a3f9bc1a1cea20b8dce7ec8d8425e - languageName: node - linkType: hard - "agent-base@npm:6": version: 6.0.2 resolution: "agent-base@npm:6.0.2" @@ -15973,6 +15949,13 @@ __metadata: languageName: node linkType: hard +"fflate@npm:^0.8.3": + version: 0.8.3 + resolution: "fflate@npm:0.8.3" + checksum: 10/6ebf528dc9c56e78e715eac615b009b25dc33e15c1920b11ebba44e6d76181c647756a81a23e19247907496b93aa99928514c53090579a65109e026ac2824aa7 + languageName: node + linkType: hard + "figures@npm:^6.1.0": version: 6.1.0 resolution: "figures@npm:6.1.0"