From a70f19d85eae673a4699cba4ca5e90c7d50128f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Fri, 7 Aug 2026 13:13:02 +0200 Subject: [PATCH 1/2] fix: extract archives with 2 GiB or more of data extractAll() read the whole data section into a single buffer with one fs.readSync() call. fs.readSync() truncates its length argument to a signed 32-bit integer, so any archive whose data section reaches 2 GiB arrives as a negative length and fails before a single file is written: RangeError [ERR_OUT_OF_RANGE]: The value of "length" is out of range. It must be >= 0. Received -348492367 at Object.readSync (node:fs:726:3) at extractAll (.../@electron/asar/lib/asar.js:245:16) asar list and asar extract-file work on the same archives, since neither goes through that read. I kept the descriptor open for the whole extraction, which is where the gain over re-opening per file actually comes from, but each entry is now copied to disk in bounded chunks instead of buffering the entire archive. Peak memory no longer scales with archive size: a 3.9 GB archive goes from failing outright to 4.2s at 201 MB peak. The copy loop also respects the fs.readSync() return value, which is allowed to be shorter than requested. The single-shot read ignored it. extractFileWithFd() takes an optional chunk size so the multi-chunk path can be covered without materialising a 2 GiB fixture. --- src/asar.ts | 109 ++++++++++++++++++++++------------------------ src/disk.ts | 44 +++++++++++++++++++ test/disk-spec.ts | 77 ++++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 56 deletions(-) diff --git a/src/asar.ts b/src/asar.ts index d60a9228..94e79fb9 100644 --- a/src/asar.ts +++ b/src/asar.ts @@ -8,6 +8,7 @@ import { BasicFilesArray, BasicStreamArray, InputMetadata, + extractFileWithFd, readArchiveHeaderSync, readFilesystemSync, readFileSync, @@ -369,68 +370,64 @@ export function extractAll(archivePath: string, dest: string) { // create destination directory fs.mkdirpSync(dest); - // Read the entire data section at once — one syscall instead of one per file. - const headerSize = filesystem.getHeaderSize(); - const archiveSize = fs.statSync(archivePath).size; - const dataStart = 8 + headerSize; - const dataSize = archiveSize - dataStart; - let dataBuf: Buffer | null = null; - if (dataSize > 0) { - dataBuf = Buffer.alloc(dataSize); - const fd = fs.openSync(archivePath, 'r'); - try { - fs.readSync(fd, dataBuf, 0, dataSize, dataStart); - } finally { - fs.closeSync(fd); - } - } + // Open the archive once and stream each entry out of it, instead of re-opening per file. + // Reading the whole data section into a single buffer is not an option: `fs.readSync()` + // truncates its `length` to a signed 32-bit int, so archives with 2 GiB or more of data + // fail with `ERR_OUT_OF_RANGE`, and smaller ones still cost their full size in memory. + const dataStart = 8 + filesystem.getHeaderSize(); + const fd = fs.openSync(archivePath, 'r'); const extractionErrors: Error[] = []; - for (const fullPath of filenames) { - // Remove leading slash - const filename = fullPath.substr(1); - const destFilename = ensureWithin(dest, filename); - const file = filesystem.getFile(filename, followLinks); - if ('files' in file) { - // it's a directory, create it and continue with the next entry - fs.mkdirpSync(destFilename); - } else if ('link' in file) { - // it's a symlink, create a symlink - const linkSrcPath = path.dirname(path.join(dest, file.link)); - const linkDestPath = path.dirname(destFilename); - const relativePath = path.relative(linkDestPath, linkSrcPath); - // try to delete output file, because we can't overwrite a link - try { - fs.unlinkSync(destFilename); - } catch {} - const linkTo = path.join(relativePath, path.basename(file.link)); - if (path.relative(dest, linkSrcPath).startsWith('..')) { - throw new Error( - `${fullPath}: file "${file.link}" links out of the package to "${linkSrcPath}"`, - ); - } - fs.symlinkSync(linkTo, destFilename); - } else { - // it's a file, try to extract it - try { - let content: Buffer; - if (file.unpacked) { - content = fs.readFileSync(path.join(`${filesystem.getRootPath()}.unpacked`, filename)); - } else if (file.size <= 0) { - content = Buffer.alloc(0); - } else { - // Slice from the pre-read data buffer — zero-copy view - const offset = parseInt(file.offset); - content = dataBuf!.subarray(offset, offset + file.size); + try { + for (const fullPath of filenames) { + // Remove leading slash + const filename = fullPath.substr(1); + const destFilename = ensureWithin(dest, filename); + const file = filesystem.getFile(filename, followLinks); + if ('files' in file) { + // it's a directory, create it and continue with the next entry + fs.mkdirpSync(destFilename); + } else if ('link' in file) { + // it's a symlink, create a symlink + const linkSrcPath = path.dirname(path.join(dest, file.link)); + const linkDestPath = path.dirname(destFilename); + const relativePath = path.relative(linkDestPath, linkSrcPath); + // try to delete output file, because we can't overwrite a link + try { + fs.unlinkSync(destFilename); + } catch {} + const linkTo = path.join(relativePath, path.basename(file.link)); + if (path.relative(dest, linkSrcPath).startsWith('..')) { + throw new Error( + `${fullPath}: file "${file.link}" links out of the package to "${linkSrcPath}"`, + ); } - fs.writeFileSync(destFilename, content); - if (file.executable) { - fs.chmodSync(destFilename, '755'); + fs.symlinkSync(linkTo, destFilename); + } else { + // it's a file, try to extract it + try { + if (file.unpacked) { + fs.writeFileSync( + destFilename, + fs.readFileSync(path.join(`${filesystem.getRootPath()}.unpacked`, filename)), + ); + } else { + const offset = parseInt(file.offset); + if (Number.isNaN(offset) || offset < 0 || !Number.isSafeInteger(offset)) { + throw new Error(`Invalid file offset in archive header: ${file.offset}`); + } + extractFileWithFd(fd, destFilename, dataStart + offset, file.size); + } + if (file.executable) { + fs.chmodSync(destFilename, '755'); + } + } catch (e) { + extractionErrors.push(e as Error); } - } catch (e) { - extractionErrors.push(e as Error); } } + } finally { + fs.closeSync(fd); } if (extractionErrors.length) { throw new Error( diff --git a/src/disk.ts b/src/disk.ts index 1335821f..e3aa9b65 100644 --- a/src/disk.ts +++ b/src/disk.ts @@ -401,6 +401,50 @@ export function readFileSync(filesystem: Filesystem, filename: string, info: Fil } } +/** + * Upper bound for a single `fs.readSync()` call when copying file contents out of an archive. + * + * `fs.readSync()` truncates its `length` argument to a signed 32-bit integer, so a single + * read of 2 GiB or more wraps to a negative value and fails with `ERR_OUT_OF_RANGE`. + * Chunking also keeps peak memory flat instead of scaling with the size of the archive. + */ +const EXTRACT_CHUNK_SIZE = 64 * 1024 * 1024; + +/** + * Copy `size` bytes starting at `position` from an already-open archive descriptor straight + * to `destPath`, without buffering the whole entry in memory. + */ +export function extractFileWithFd( + fd: number, + destPath: string, + position: number, + size: number, + chunkSize: number = EXTRACT_CHUNK_SIZE, +) { + const out = fs.openSync(destPath, 'w'); + try { + if (size <= 0) { + return; + } + const buffer = Buffer.alloc(Math.min(size, chunkSize)); + let copied = 0; + while (copied < size) { + const wanted = Math.min(buffer.length, size - copied); + // `readSync` is allowed to return fewer bytes than requested, so always loop on the result. + const read = fs.readSync(fd, buffer, 0, wanted, position + copied); + if (read <= 0) { + throw new Error( + `Unexpected end of archive while extracting "${destPath}" (read ${copied} of ${size} bytes)`, + ); + } + fs.writeSync(out, buffer, 0, read); + copied += read; + } + } finally { + fs.closeSync(out); + } +} + export function readFileWithFd( fd: number, filesystem: Filesystem, diff --git a/test/disk-spec.ts b/test/disk-spec.ts index 1a2c3d3f..e9d6fa99 100644 --- a/test/disk-spec.ts +++ b/test/disk-spec.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { createPackage, getRawHeader, uncacheAll } from '../src/asar.js'; import { + extractFileWithFd, readArchiveHeaderSync, readFilesystemSync, readFileSync, @@ -319,4 +320,80 @@ describe('disk', () => { expect(() => readFileWithFd(-1, filesystem, '../outside.txt', info)).toThrow('outside'); }); }); + + describe('extractFileWithFd', () => { + /** + * `fs.readSync()` truncates its `length` argument to a signed 32-bit integer, so entries + * of 2 GiB or more cannot be copied with a single read. Materialising such an archive in + * a test is impractical, so the chunk size is lowered instead to exercise the same loop. + */ + const writeSource = (dir: string, name: string, contents: Buffer) => { + const srcPath = path.join(dir, name); + fs.writeFileSync(srcPath, contents); + return srcPath; + }; + + it('copies contents that span multiple chunks', () => { + const dir = tmpDir('extract-multi-chunk'); + const payload = Buffer.from('the quick brown fox jumps over the lazy dog'); + const prefix = Buffer.from('HEADER'); + const srcPath = writeSource(dir, 'multi-chunk.bin', Buffer.concat([prefix, payload])); + const destPath = path.join(dir, 'multi-chunk.out'); + + const fd = fs.openSync(srcPath, 'r'); + try { + extractFileWithFd(fd, destPath, prefix.length, payload.length, 7); + } finally { + fs.closeSync(fd); + } + + expect(fs.readFileSync(destPath).equals(payload)).toBe(true); + }); + + it('copies contents smaller than a single chunk', () => { + const dir = tmpDir('extract-small'); + const payload = Buffer.from('short'); + const srcPath = writeSource(dir, 'small.bin', payload); + const destPath = path.join(dir, 'small.out'); + + const fd = fs.openSync(srcPath, 'r'); + try { + extractFileWithFd(fd, destPath, 0, payload.length, 1024); + } finally { + fs.closeSync(fd); + } + + expect(fs.readFileSync(destPath).equals(payload)).toBe(true); + }); + + it('creates an empty file for zero-length entries', () => { + const dir = tmpDir('extract-empty'); + const srcPath = writeSource(dir, 'empty-src.bin', Buffer.from('ignored')); + const destPath = path.join(dir, 'empty.out'); + + const fd = fs.openSync(srcPath, 'r'); + try { + extractFileWithFd(fd, destPath, 0, 0); + } finally { + fs.closeSync(fd); + } + + expect(fs.readFileSync(destPath).length).toBe(0); + }); + + it('throws when the archive ends before the entry does', () => { + const dir = tmpDir('extract-truncated'); + const srcPath = writeSource(dir, 'truncated.bin', Buffer.from('only ten b')); + const destPath = path.join(dir, 'truncated.out'); + + const fd = fs.openSync(srcPath, 'r'); + try { + expect(() => extractFileWithFd(fd, destPath, 0, 1000, 4)).toThrow( + /Unexpected end of archive/, + ); + } finally { + fs.closeSync(fd); + } + }); + }); }); From 1a562495a911b902f6e88081d1e4fb0d7cac0c8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Fri, 7 Aug 2026 13:13:15 +0200 Subject: [PATCH 2/2] fix: constrain unpacked reads in extractAll to the unpacked directory readFileSync() in disk.ts already resolves unpacked entries through ensureWithin(). extractAll() joined the path directly instead, so the two disagreed on the same input. This is separate from the 2 GiB read fix and can be dropped on its own. --- src/asar.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/asar.ts b/src/asar.ts index 94e79fb9..9294fab3 100644 --- a/src/asar.ts +++ b/src/asar.ts @@ -407,10 +407,8 @@ export function extractAll(archivePath: string, dest: string) { // it's a file, try to extract it try { if (file.unpacked) { - fs.writeFileSync( - destFilename, - fs.readFileSync(path.join(`${filesystem.getRootPath()}.unpacked`, filename)), - ); + const unpackedDir = `${filesystem.getRootPath()}.unpacked`; + fs.writeFileSync(destFilename, fs.readFileSync(ensureWithin(unpackedDir, filename))); } else { const offset = parseInt(file.offset); if (Number.isNaN(offset) || offset < 0 || !Number.isSafeInteger(offset)) {