From dd4234ad9fd7f8c0cabfeb9083b0233c1a4d8da4 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Tue, 11 Aug 2026 00:53:51 +0530 Subject: [PATCH 1/4] fix(cli-upload): drop image-size for a built-in PNG/JPEG reader (PER-10427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `image-size` is archived upstream and carries three unfixable high-severity advisories (CVE-2025-71329, CVE-2025-71330), so `npm audit` fails for anyone who installs @percy/cli. There is no patched version to move to — every published release through 2.0.2 is affected — and 2.x would also undo the Node 14 support that #2301 pinned ~1.0.2 to keep. The command only ever accepts png, jpg and jpeg (ALLOWED_FILE_TYPES), so a general purpose image parser was always more surface than this needed. Reading the two formats we actually support is about eighty lines and removes the dependency outright. The advisories were reachable here, not just theoretical: `image-size` picks its parser from magic bytes while `percy upload` filters on extension, so an ICNS buffer named `.png` reached the ICNS parser and wedged the event loop — `percy upload` hung indefinitely and did not respond to SIGTERM. Such a file is now skipped with a log line. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli-upload/package.json | 3 +- packages/cli-upload/src/image-size.js | 104 ++++++++++++++++++ packages/cli-upload/src/upload.js | 11 +- packages/cli-upload/test/fixtures.js | 90 +++++++++++++++ .../cli-upload/test/unit/image-size.test.js | 90 +++++++++++++++ packages/cli-upload/test/upload.test.js | 47 ++++++-- yarn.lock | 14 --- 7 files changed, 332 insertions(+), 27 deletions(-) create mode 100644 packages/cli-upload/src/image-size.js create mode 100644 packages/cli-upload/test/fixtures.js create mode 100644 packages/cli-upload/test/unit/image-size.test.js diff --git a/packages/cli-upload/package.json b/packages/cli-upload/package.json index 62ac41307..766638660 100644 --- a/packages/cli-upload/package.json +++ b/packages/cli-upload/package.json @@ -34,7 +34,6 @@ }, "dependencies": { "@percy/cli-command": "1.32.7-beta.0", - "fast-glob": "^3.2.11", - "image-size": "~1.0.2" + "fast-glob": "^3.2.11" } } diff --git a/packages/cli-upload/src/image-size.js b/packages/cli-upload/src/image-size.js new file mode 100644 index 000000000..4608d2a91 --- /dev/null +++ b/packages/cli-upload/src/image-size.js @@ -0,0 +1,104 @@ +import fs from 'fs'; + +// Minimal PNG/JPEG dimension reader. `percy upload` only ever accepts png, jpg and +// jpeg files (see ALLOWED_FILE_TYPES in upload.js), so a general purpose image +// parser is more dependency — and more attack surface — than this command needs. +// The previous dependency (`image-size`) is archived upstream and carries +// unfixable infinite-loop advisories in parsers we never wanted in the first +// place (CVE-2025-71329, CVE-2025-71330). + +const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const IHDR = Buffer.from('IHDR', 'ascii'); + +// SOFn markers carry frame dimensions. 0xc4 (DHT), 0xc8 (JPG) and 0xcc (DAC) +// sit in the same range but are not frame headers. +const JPEG_SOF_MARKERS = new Set([ + 0xc0, 0xc1, 0xc2, 0xc3, 0xc5, 0xc6, 0xc7, + 0xc9, 0xca, 0xcb, 0xcd, 0xce, 0xcf +]); + +// Markers that stand alone — no length-prefixed payload follows them. +const JPEG_STANDALONE_MARKERS = new Set([ + 0x01, 0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7, 0xd8 +]); + +// Reads exactly `length` bytes at `position`, or returns null on a short read. +function readAt(fd, length, position) { + let buffer = Buffer.alloc(length); + let bytesRead = fs.readSync(fd, buffer, 0, length, position); + return bytesRead === length ? buffer : null; +} + +// signature (8) + chunk length (4) + chunk type (4) + width (4) + height (4) +function pngSize(fd) { + let header = readAt(fd, 24, 0); + if (!header?.subarray(12, 16).equals(IHDR)) return null; + + return { + width: header.readUInt32BE(16), + height: header.readUInt32BE(20) + }; +} + +function jpegSize(fd, fileSize) { + // start just past the SOI marker + let offset = 2; + + while (offset + 4 <= fileSize) { + let header = readAt(fd, 4, offset); + // every marker begins with 0xff — anything else means we've walked out of + // the segment chain and into entropy-coded data + if (header?.[0] !== 0xff) return null; + + let marker = header[1]; + // 0xff may repeat as fill bytes before the marker itself + if (marker === 0xff) { offset += 1; continue; } + if (JPEG_STANDALONE_MARKERS.has(marker)) { offset += 2; continue; } + // SOS begins scan data and EOI ends the image — a frame header should + // already have been seen by now, so there is nothing left to find + if (marker === 0xda || marker === 0xd9) return null; + + let length = header.readUInt16BE(2); + // A segment length always counts its own two length bytes. Anything shorter + // is malformed, and advancing by it would not move `offset` forward — the + // exact shape of the infinite loops that made `image-size` unfixable. + if (length < 2) return null; + + if (JPEG_SOF_MARKERS.has(marker)) { + // precision (1) + height (2) + width (2) + let frame = readAt(fd, 5, offset + 4); + if (!frame) return null; + + return { + width: frame.readUInt16BE(3), + height: frame.readUInt16BE(1) + }; + } + + offset += 2 + length; + } + + return null; +} + +// Returns `{ width, height }` for a PNG or JPEG file, or null when the file is +// neither — including when its extension disagrees with its actual contents. +export function imageSize(absolutePath) { + let fd = fs.openSync(absolutePath, 'r'); + + try { + let signature = readAt(fd, 8, 0); + if (!signature) return null; + + if (signature.equals(PNG_SIGNATURE)) return pngSize(fd); + if (signature[0] === 0xff && signature[1] === 0xd8) { + return jpegSize(fd, fs.fstatSync(fd).size); + } + + return null; + } finally { + fs.closeSync(fd); + } +} + +export default imageSize; diff --git a/packages/cli-upload/src/upload.js b/packages/cli-upload/src/upload.js index abd3106fe..d2fa6a65f 100644 --- a/packages/cli-upload/src/upload.js +++ b/packages/cli-upload/src/upload.js @@ -85,7 +85,7 @@ export const upload = command('upload', { exit(1, 'Invalid Token Type. Only "web" and "self-managed" token types are allowed.'); } - let { default: imageSize } = await import('image-size'); + let { imageSize } = await import('./image-size.js'); let { getImageResources } = await import('./utils.js'); // the internal discovery queue shares a concurrency with the snapshots queue @@ -97,7 +97,14 @@ export const upload = command('upload', { log.info(`Skipping unsupported file type: ${relativePath}`); } else { let absolutePath = path.resolve(args.dirname, relativePath); - let img = { relativePath, absolutePath, ...imageSize(absolutePath) }; + let size = imageSize(absolutePath); + + if (!size) { + log.info(`Skipping file with unreadable image data: ${relativePath}`); + continue; + } + + let img = { relativePath, absolutePath, ...size }; let { dir, name, ext } = path.parse(relativePath); img.type = ext === '.png' ? 'png' : 'jpeg'; img.name = path.join(dir, name); diff --git a/packages/cli-upload/test/fixtures.js b/packages/cli-upload/test/fixtures.js new file mode 100644 index 000000000..6fd4379af --- /dev/null +++ b/packages/cli-upload/test/fixtures.js @@ -0,0 +1,90 @@ +// Real image bytes, kept as buffers rather than strings — the PNG signature +// starts with 0x89, which does not survive a round trip through UTF-8. + +const b64 = str => Buffer.from(str, 'base64'); + +// 1x1 red PNG +export const PNG_PIXEL = b64( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ' + + '/pLvAAAAAElFTkSuQmCC' +); + +// 120x80 red PNG +export const PNG_120X80 = b64( + 'iVBORw0KGgoAAAANSUhEUgAAAHgAAABQCAIAAABd+SbeAAAA4klEQVR4nO3OQQ0AIAADsfk3' + + 'DS7o40gqoDvb94AfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4' + + 'QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E' + + '+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQf' + + 'RPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRFwNVlys/6lZ' + + 'IQAAAABJRU5ErkJggg==' +); + +// 1x1 red JPEG +export const JPEG_PIXEL = b64( + '/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9' + + 'PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhC' + + 'Y2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAAR' + + 'CAABAAEDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA' + + 'AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK' + + 'FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG' + + 'h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl' + + '5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA' + + 'AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk' + + 'NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE' + + 'hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk' + + '5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwDFoooryz7w/9k=' +); + +// 200x150 red JPEG — its frame header sits past several kilobyte-scale +// quantization and Huffman tables, so reading it exercises segment walking +// rather than a fixed header offset. +export const JPEG_200X150 = b64( + '/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9' + + 'PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhC' + + 'Y2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAAR' + + 'CACWAMgDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA' + + 'AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK' + + 'FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG' + + 'h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl' + + '5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA' + + 'AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk' + + 'NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE' + + 'hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk' + + '5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwDFoooryz7wKKKKACiiigAooooAKKKKACii' + + 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + + 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + + 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + + 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + + 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + + 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + + 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + + 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + + 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA/9k=' +); + +// A GIF — accepted by neither the extension filter nor the size reader. +export const GIF_PIXEL = b64('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); + +// An ICNS buffer with valid magic bytes and a zero-valued entry length. +// This is the CVE-2025-71330 proof of concept: the archived `image-size` +// package looped forever on it because a zero-length entry never advanced the +// read offset. `percy upload` filters by extension, not magic bytes, so a file +// named `.png` reached that parser. +export function icnsZeroLengthEntry() { + let buffer = Buffer.alloc(64); + buffer.write('icns', 0, 'ascii'); + buffer.writeUInt32BE(64, 4); // file length + buffer.write('ic09', 8, 'ascii'); // first entry type + buffer.writeUInt32BE(0, 12); // first entry length + return buffer; +} + +// A JPEG whose first segment declares a length of zero. Advancing by a +// self-inclusive length below 2 would leave the read offset stationary. +export function jpegZeroLengthSegment() { + let buffer = Buffer.alloc(32); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffe0, 2); // APP0 + buffer.writeUInt16BE(0, 4); // segment length + return buffer; +} diff --git a/packages/cli-upload/test/unit/image-size.test.js b/packages/cli-upload/test/unit/image-size.test.js new file mode 100644 index 000000000..fd3ac15e1 --- /dev/null +++ b/packages/cli-upload/test/unit/image-size.test.js @@ -0,0 +1,90 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { imageSize } from '../../src/image-size.js'; +import { + PNG_PIXEL, + PNG_120X80, + JPEG_PIXEL, + JPEG_200X150, + GIF_PIXEL, + icnsZeroLengthEntry, + jpegZeroLengthSegment +} from '../fixtures.js'; + +describe('unit / image-size', () => { + let dirname, index = 0; + + // these tests read real files — `imageSize` opens a descriptor and reads at + // offsets, which is the behaviour worth exercising against a real filesystem + beforeAll(() => { + dirname = fs.mkdtempSync(path.join(os.tmpdir(), 'percy-image-size-')); + }); + + afterAll(() => { + fs.rmSync(dirname, { recursive: true, force: true }); + }); + + let write = contents => { + let filename = path.join(dirname, `fixture-${index++}`); + fs.writeFileSync(filename, contents); + return filename; + }; + + it('reads PNG dimensions', () => { + expect(imageSize(write(PNG_PIXEL))).toEqual({ width: 1, height: 1 }); + expect(imageSize(write(PNG_120X80))).toEqual({ width: 120, height: 80 }); + }); + + it('reads JPEG dimensions', () => { + expect(imageSize(write(JPEG_PIXEL))).toEqual({ width: 1, height: 1 }); + expect(imageSize(write(JPEG_200X150))).toEqual({ width: 200, height: 150 }); + }); + + it('returns null for other image formats', () => { + expect(imageSize(write(GIF_PIXEL))).toBeNull(); + }); + + it('returns null for files that are not images', () => { + expect(imageSize(write('not an image'))).toBeNull(); + expect(imageSize(write(Buffer.alloc(0)))).toBeNull(); + }); + + it('returns null for a truncated PNG', () => { + expect(imageSize(write(PNG_PIXEL.subarray(0, 16)))).toBeNull(); + }); + + it('returns null for a PNG whose first chunk is not IHDR', () => { + let png = Buffer.from(PNG_PIXEL); + png.write('IDAT', 12, 'ascii'); + expect(imageSize(write(png))).toBeNull(); + }); + + it('returns null for a JPEG with no frame header', () => { + // truncate to the SOI marker and the start of APP0 + expect(imageSize(write(JPEG_PIXEL.subarray(0, 4)))).toBeNull(); + }); + + // CVE-2025-71330 / CVE-2025-71329 — the advisories that made `image-size` + // unusable were all the same shape: a zero-valued length field left the read + // offset unchanged, so the parser looped forever and wedged the event loop. + it('terminates on an ICNS buffer with a zero-length entry', () => { + // named `.png` because `percy upload` filters on extension, which is how + // this buffer reached the ICNS parser in the first place + let filename = path.join(dirname, 'crafted.png'); + fs.writeFileSync(filename, icnsZeroLengthEntry()); + + expect(imageSize(filename)).toBeNull(); + }); + + it('terminates on a JPEG segment with a zero length', () => { + expect(imageSize(write(jpegZeroLengthSegment()))).toBeNull(); + }); + + it('terminates on a JPEG of nothing but 0xff fill bytes', () => { + let buffer = Buffer.alloc(4096, 0xff); + buffer.writeUInt16BE(0xffd8, 0); + + expect(imageSize(write(buffer))).toBeNull(); + }); +}); diff --git a/packages/cli-upload/test/upload.test.js b/packages/cli-upload/test/upload.test.js index c66af5c92..1089c92f3 100644 --- a/packages/cli-upload/test/upload.test.js +++ b/packages/cli-upload/test/upload.test.js @@ -1,11 +1,7 @@ import { fs, logger, api, setupTest } from '@percy/cli-command/test/helpers'; import upload from '@percy/cli-upload'; import { BYOS_TAG } from '../src/upload.js'; - -// http://png-pixel.com/ -const pixel = Buffer.from(( - 'R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==' -), 'base64').toString(); +import { PNG_PIXEL, JPEG_PIXEL, GIF_PIXEL, icnsZeroLengthEntry } from './fixtures.js'; describe('percy upload', () => { beforeEach(async () => { @@ -15,13 +11,18 @@ describe('percy upload', () => { process.env.PERCY_FORCE_PKG_VALUE = JSON.stringify({ name: '@percy/client', version: '1.0.0' }); await setupTest({ filesystem: { - 'images/test-1.png': pixel, - 'images/test-2.jpg': pixel, - 'images/test-3.jpeg': pixel, - 'images/test-4.gif': pixel, + 'images/.keep': '', './nope': 'not here' } }); + + // written as buffers rather than through `filesystem` above, which only + // creates files from strings — and image bytes are not valid UTF-8 + fs.writeFileSync('images/test-1.png', PNG_PIXEL); + fs.writeFileSync('images/test-2.jpg', JPEG_PIXEL); + fs.writeFileSync('images/test-3.jpeg', JPEG_PIXEL); + fs.writeFileSync('images/test-4.gif', GIF_PIXEL); + fs.unlinkSync('images/.keep'); }); afterEach(() => { @@ -157,6 +158,34 @@ describe('percy upload', () => { ])); }); + it('skips files whose contents are not a readable image', async () => { + fs.writeFileSync('images/test-5.png', 'this is not a png'); + await upload(['./images']); + + expect(logger.stderr).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Skipping file with unreadable image data: test-5.png', + '[percy] Uploading 3 snapshots...', + '[percy] Snapshot uploaded: test-1.png' + ])); + }); + + // Regression for CVE-2025-71330. The previous `image-size` dependency picked + // its parser from magic bytes while this command filters on extension, so an + // ICNS buffer named `.png` reached a parser that looped forever on it. The + // upload now completes and the crafted file is skipped. + it('skips a crafted ICNS file named as a png without hanging', async () => { + fs.writeFileSync('images/crafted.png', icnsZeroLengthEntry()); + await upload(['./images']); + + expect(logger.stderr).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Skipping file with unreadable image data: crafted.png', + '[percy] Uploading 3 snapshots...', + '[percy] Finalized build #1: https://percy.io/test/test/123' + ])); + }); + it('does not upload snapshots and prints matching files with --dry-run', async () => { await upload(['./images', '--dry-run']); diff --git a/yarn.lock b/yarn.lock index 46b0f213e..86fe00efb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5468,13 +5468,6 @@ ignore@^5.0.4, ignore@^5.1.1, ignore@^5.2.0: resolved "https://registry.npmjs.org/ignore/-/ignore-5.2.0.tgz" integrity sha512-CmxgYGiEPCLhfLnpPp1MoRmifwEIOgjcHXxOBjv7mY96c+eWScsOP9c112ZyLdWHi0FxHjI+4uVhKYp/gcdRmQ== -image-size@~1.0.2: - version "1.0.2" - resolved "https://registry.npmjs.org/image-size/-/image-size-1.0.2.tgz" - integrity sha512-xfOoWjceHntRb3qFCrh5ZFORYH8XCdYpASltMhZ/Q0KZiOwjdE/Yl2QCiWdwD+lygV5bMCvauzgu5PxBX/Yerg== - dependencies: - queue "6.0.2" - import-fresh@^3.0.0, import-fresh@^3.2.1: version "3.3.0" resolved "https://registry.npmjs.org/import-fresh/-/import-fresh-3.3.0.tgz" @@ -7778,13 +7771,6 @@ queue-microtask@^1.2.2: resolved "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz" integrity sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A== -queue@6.0.2: - version "6.0.2" - resolved "https://registry.npmjs.org/queue/-/queue-6.0.2.tgz" - integrity sha512-iHZWu+q3IdFZFX36ro/lKBkSvfkztY5Y7HMiPlOUjhupPcG2JMfst2KKEpu5XndviX/3UhFbRngUPNKtgvtZiA== - dependencies: - inherits "~2.0.3" - quick-lru@^4.0.1: version "4.0.1" resolved "https://registry.npmjs.org/quick-lru/-/quick-lru-4.0.1.tgz" From cb6ad32edd1a47ebdc9d170cbef8c46902dff169 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Tue, 11 Aug 2026 09:36:43 +0530 Subject: [PATCH 2/4] test(cli-upload): cover the JPEG walk's malformed-input branches CI enforces 100% coverage; image-size.js left branches 51, 56-59 and 70 unhit. Adds fixtures for the four segment-walk exits that had no test: walking off the chain into non-marker data, a standalone marker preceding the frame header, SOS/EOI reached before any frame, and a file that ends before the frame payload it announced. Also drops the optional chaining on the marker read. The loop bound `offset + 4 <= fileSize` already proves those four bytes exist, so the null arm was unreachable and could never be covered. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli-upload/src/image-size.js | 3 +- packages/cli-upload/test/fixtures.js | 53 +++++++++++++++++++ .../cli-upload/test/unit/image-size.test.js | 31 +++++++++-- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/cli-upload/src/image-size.js b/packages/cli-upload/src/image-size.js index 4608d2a91..9594a04f8 100644 --- a/packages/cli-upload/src/image-size.js +++ b/packages/cli-upload/src/image-size.js @@ -45,10 +45,11 @@ function jpegSize(fd, fileSize) { let offset = 2; while (offset + 4 <= fileSize) { + // the loop bound guarantees these four bytes exist, so this cannot short read let header = readAt(fd, 4, offset); // every marker begins with 0xff — anything else means we've walked out of // the segment chain and into entropy-coded data - if (header?.[0] !== 0xff) return null; + if (header[0] !== 0xff) return null; let marker = header[1]; // 0xff may repeat as fill bytes before the marker itself diff --git a/packages/cli-upload/test/fixtures.js b/packages/cli-upload/test/fixtures.js index 6fd4379af..bc08e0c51 100644 --- a/packages/cli-upload/test/fixtures.js +++ b/packages/cli-upload/test/fixtures.js @@ -88,3 +88,56 @@ export function jpegZeroLengthSegment() { buffer.writeUInt16BE(0, 4); // segment length return buffer; } + +// A JPEG whose first segment length lands the walk on bytes that do not begin a +// marker — i.e. off the segment chain and into data. +export function jpegWalksIntoData() { + let buffer = Buffer.alloc(32); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffe0, 2); // APP0 + buffer.writeUInt16BE(4, 4); // length 4 ⇒ next marker expected at offset 8 + // offset 8 onward is left zeroed, so no 0xff marker prefix is found + return buffer; +} + +// A JPEG carrying a standalone marker (RST0, no length payload) ahead of the +// frame header. Mis-skipping it would desynchronise the walk. +export function jpegStandaloneMarkerBeforeFrame(width, height) { + let buffer = Buffer.alloc(16); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffd0, 2); // RST0 — standalone + buffer.writeUInt16BE(0xffc0, 4); // SOF0 + buffer.writeUInt16BE(11, 6); // segment length + buffer.writeUInt8(8, 8); // sample precision + buffer.writeUInt16BE(height, 9); + buffer.writeUInt16BE(width, 11); + return buffer; +} + +// A JPEG that reaches the start of scan without ever declaring a frame. +export function jpegScanWithoutFrame() { + let buffer = Buffer.alloc(32); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffda, 2); // SOS + buffer.writeUInt16BE(12, 4); // segment length + return buffer; +} + +// A JPEG that reaches end of image without ever declaring a frame. +export function jpegEndsWithoutFrame() { + let buffer = Buffer.alloc(32); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffd9, 2); // EOI + return buffer; +} + +// A JPEG that announces a frame header and then ends before it. Must stay at +// least 8 bytes long, or it is rejected at the signature read and never reaches +// the segment walk this is meant to exercise. +export function jpegTruncatedFrameHeader() { + let buffer = Buffer.alloc(8); + buffer.writeUInt16BE(0xffd8, 0); // SOI + buffer.writeUInt16BE(0xffc0, 2); // SOF0 + buffer.writeUInt16BE(11, 4); // length claims a payload the file does not have + return buffer; +} diff --git a/packages/cli-upload/test/unit/image-size.test.js b/packages/cli-upload/test/unit/image-size.test.js index fd3ac15e1..062da71e7 100644 --- a/packages/cli-upload/test/unit/image-size.test.js +++ b/packages/cli-upload/test/unit/image-size.test.js @@ -9,7 +9,12 @@ import { JPEG_200X150, GIF_PIXEL, icnsZeroLengthEntry, - jpegZeroLengthSegment + jpegZeroLengthSegment, + jpegWalksIntoData, + jpegStandaloneMarkerBeforeFrame, + jpegScanWithoutFrame, + jpegEndsWithoutFrame, + jpegTruncatedFrameHeader } from '../fixtures.js'; describe('unit / image-size', () => { @@ -60,11 +65,31 @@ describe('unit / image-size', () => { expect(imageSize(write(png))).toBeNull(); }); - it('returns null for a JPEG with no frame header', () => { - // truncate to the SOI marker and the start of APP0 + it('returns null for a JPEG too short to hold a signature', () => { expect(imageSize(write(JPEG_PIXEL.subarray(0, 4)))).toBeNull(); }); + it('returns null when the segment chain leads into non-marker data', () => { + expect(imageSize(write(jpegWalksIntoData()))).toBeNull(); + }); + + it('returns null for a JPEG that starts its scan without a frame', () => { + expect(imageSize(write(jpegScanWithoutFrame()))).toBeNull(); + }); + + it('returns null for a JPEG that ends without a frame', () => { + expect(imageSize(write(jpegEndsWithoutFrame()))).toBeNull(); + }); + + it('returns null for a JPEG that ends before its frame header', () => { + expect(imageSize(write(jpegTruncatedFrameHeader()))).toBeNull(); + }); + + it('skips standalone markers to reach the frame header', () => { + expect(imageSize(write(jpegStandaloneMarkerBeforeFrame(320, 240)))) + .toEqual({ width: 320, height: 240 }); + }); + // CVE-2025-71330 / CVE-2025-71329 — the advisories that made `image-size` // unusable were all the same shape: a zero-valued length field left the read // offset unchanged, so the parser looped forever and wedged the event loop. From 1b6fb4b62c6f847c742507a579a00a87ef74f9d0 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 17 Aug 2026 18:50:57 +0530 Subject: [PATCH 3/4] fix(cli-upload): read dimensions with probe-image-size instead of a built-in parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hand-rolled PNG/JPEG reader with `probe-image-size`, so the package depends on a maintained parser rather than one we own. `probe-image-size` has no ICNS, JXL or HEIF parser at all, and its ISOBMFF reader rejects a box smaller than its own header rather than advancing by it — the shape of every advisory that made `image-size` unfixable. Its tree is advisory-free, it is pure ES5 so Node 14 is unaffected, and 7.4.0 is current. Only the `sync.js` entrypoint is imported, which pulls in the buffer parsers and none of the http or stream machinery. The extension is required because the package publishes no `exports` map and this package is ESM. Two details worth noting: - Reads a bounded 512 KiB prefix, matching the `MaxBufferSize` that `image-size` applied, so no file that used to be readable becomes unreadable. - Gates on the reported type, because the parser reads about ten formats while `upload` accepts only png and jpeg. A GIF named `.png` is still skipped. `jpegStandaloneMarkerBeforeFrame` declared an 11-byte SOF0 segment inside a 16-byte buffer, one byte past the end. The previous reader returned dimensions anyway because it never checked the declared length against the bytes present; this one does, so the fixture is now a valid single-component frame header rather than a truncated one. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli-upload/package.json | 3 +- packages/cli-upload/src/image-size.js | 129 +++++------------- packages/cli-upload/test/fixtures.js | 11 +- .../cli-upload/test/unit/image-size.test.js | 8 +- yarn.lock | 36 ++++- 5 files changed, 86 insertions(+), 101 deletions(-) diff --git a/packages/cli-upload/package.json b/packages/cli-upload/package.json index 766638660..e82be7b36 100644 --- a/packages/cli-upload/package.json +++ b/packages/cli-upload/package.json @@ -34,6 +34,7 @@ }, "dependencies": { "@percy/cli-command": "1.32.7-beta.0", - "fast-glob": "^3.2.11" + "fast-glob": "^3.2.11", + "probe-image-size": "^7.4.0" } } diff --git a/packages/cli-upload/src/image-size.js b/packages/cli-upload/src/image-size.js index 9594a04f8..e41fa99fd 100644 --- a/packages/cli-upload/src/image-size.js +++ b/packages/cli-upload/src/image-size.js @@ -1,105 +1,46 @@ import fs from 'fs'; +// the `sync.js` entrypoint pulls in only the buffer parsers — none of the http +// or stream machinery, and so none of `needle`. Extension included because the +// package publishes no `exports` map, and ESM will not resolve it without one. +import probeSync from 'probe-image-size/sync.js'; + +// `percy upload` only ever accepts png, jpg and jpeg files (see ALLOWED_FILE_TYPES +// in upload.js), so anything else is rejected here even though the parser can read +// it. The previous dependency (`image-size`) is archived upstream and carries +// unfixable infinite-loop advisories (CVE-2025-71329, CVE-2025-71330) in parsers +// this command never wanted. `probe-image-size` has no ICNS, JXL or HEIF parser at +// all, and its ISOBMFF reader rejects a box smaller than its own header rather than +// advancing by it — the shape of every one of those advisories. +const SUPPORTED_TYPES = new Set(['png', 'jpg']); + +// A JPEG frame header sits behind however much metadata the encoder wrote, so the +// whole header cannot be read at a fixed offset. This is the limit `image-size` +// applied for the same reason, kept so that files it could read stay readable. +const MAX_HEADER_BYTES = 512 * 1024; + +// Reads the leading bytes of a file without pulling a multi-megabyte image into +// memory just to read its dimensions. +function readHeader(absolutePath) { + let fd = fs.openSync(absolutePath, 'r'); -// Minimal PNG/JPEG dimension reader. `percy upload` only ever accepts png, jpg and -// jpeg files (see ALLOWED_FILE_TYPES in upload.js), so a general purpose image -// parser is more dependency — and more attack surface — than this command needs. -// The previous dependency (`image-size`) is archived upstream and carries -// unfixable infinite-loop advisories in parsers we never wanted in the first -// place (CVE-2025-71329, CVE-2025-71330). - -const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); -const IHDR = Buffer.from('IHDR', 'ascii'); - -// SOFn markers carry frame dimensions. 0xc4 (DHT), 0xc8 (JPG) and 0xcc (DAC) -// sit in the same range but are not frame headers. -const JPEG_SOF_MARKERS = new Set([ - 0xc0, 0xc1, 0xc2, 0xc3, 0xc5, 0xc6, 0xc7, - 0xc9, 0xca, 0xcb, 0xcd, 0xce, 0xcf -]); - -// Markers that stand alone — no length-prefixed payload follows them. -const JPEG_STANDALONE_MARKERS = new Set([ - 0x01, 0xd0, 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7, 0xd8 -]); - -// Reads exactly `length` bytes at `position`, or returns null on a short read. -function readAt(fd, length, position) { - let buffer = Buffer.alloc(length); - let bytesRead = fs.readSync(fd, buffer, 0, length, position); - return bytesRead === length ? buffer : null; -} - -// signature (8) + chunk length (4) + chunk type (4) + width (4) + height (4) -function pngSize(fd) { - let header = readAt(fd, 24, 0); - if (!header?.subarray(12, 16).equals(IHDR)) return null; - - return { - width: header.readUInt32BE(16), - height: header.readUInt32BE(20) - }; -} - -function jpegSize(fd, fileSize) { - // start just past the SOI marker - let offset = 2; - - while (offset + 4 <= fileSize) { - // the loop bound guarantees these four bytes exist, so this cannot short read - let header = readAt(fd, 4, offset); - // every marker begins with 0xff — anything else means we've walked out of - // the segment chain and into entropy-coded data - if (header[0] !== 0xff) return null; - - let marker = header[1]; - // 0xff may repeat as fill bytes before the marker itself - if (marker === 0xff) { offset += 1; continue; } - if (JPEG_STANDALONE_MARKERS.has(marker)) { offset += 2; continue; } - // SOS begins scan data and EOI ends the image — a frame header should - // already have been seen by now, so there is nothing left to find - if (marker === 0xda || marker === 0xd9) return null; - - let length = header.readUInt16BE(2); - // A segment length always counts its own two length bytes. Anything shorter - // is malformed, and advancing by it would not move `offset` forward — the - // exact shape of the infinite loops that made `image-size` unfixable. - if (length < 2) return null; - - if (JPEG_SOF_MARKERS.has(marker)) { - // precision (1) + height (2) + width (2) - let frame = readAt(fd, 5, offset + 4); - if (!frame) return null; - - return { - width: frame.readUInt16BE(3), - height: frame.readUInt16BE(1) - }; - } - - offset += 2 + length; + try { + let length = Math.min(fs.fstatSync(fd).size, MAX_HEADER_BYTES); + let buffer = Buffer.alloc(length); + // trailing bytes are only unwritten if the file shrank mid-read, but slicing + // to the actual count is correct either way and keeps this branchless + let bytesRead = fs.readSync(fd, buffer, 0, length, 0); + return buffer.subarray(0, bytesRead); + } finally { + fs.closeSync(fd); } - - return null; } // Returns `{ width, height }` for a PNG or JPEG file, or null when the file is // neither — including when its extension disagrees with its actual contents. export function imageSize(absolutePath) { - let fd = fs.openSync(absolutePath, 'r'); - - try { - let signature = readAt(fd, 8, 0); - if (!signature) return null; - - if (signature.equals(PNG_SIGNATURE)) return pngSize(fd); - if (signature[0] === 0xff && signature[1] === 0xd8) { - return jpegSize(fd, fs.fstatSync(fd).size); - } - - return null; - } finally { - fs.closeSync(fd); - } + let result = probeSync(readHeader(absolutePath)); + if (!result || !SUPPORTED_TYPES.has(result.type)) return null; + return { width: result.width, height: result.height }; } export default imageSize; diff --git a/packages/cli-upload/test/fixtures.js b/packages/cli-upload/test/fixtures.js index bc08e0c51..f7a16af90 100644 --- a/packages/cli-upload/test/fixtures.js +++ b/packages/cli-upload/test/fixtures.js @@ -103,7 +103,12 @@ export function jpegWalksIntoData() { // A JPEG carrying a standalone marker (RST0, no length payload) ahead of the // frame header. Mis-skipping it would desynchronise the walk. export function jpegStandaloneMarkerBeforeFrame(width, height) { - let buffer = Buffer.alloc(16); + // A SOF0 declaring one component is 11 bytes: length (2) + precision (1) + + // dimensions (4) + component count (1) + one component spec (3). The segment + // starts at offset 6, so the buffer has to reach offset 17 for the length it + // declares to be satisfiable — a parser that checks the declared length + // against the bytes actually present rejects anything shorter. + let buffer = Buffer.alloc(17); buffer.writeUInt16BE(0xffd8, 0); // SOI buffer.writeUInt16BE(0xffd0, 2); // RST0 — standalone buffer.writeUInt16BE(0xffc0, 4); // SOF0 @@ -111,6 +116,10 @@ export function jpegStandaloneMarkerBeforeFrame(width, height) { buffer.writeUInt8(8, 8); // sample precision buffer.writeUInt16BE(height, 9); buffer.writeUInt16BE(width, 11); + buffer.writeUInt8(1, 13); // component count + buffer.writeUInt8(1, 14); // component id + buffer.writeUInt8(0x11, 15); // sampling factors + buffer.writeUInt8(0, 16); // quantization table selector return buffer; } diff --git a/packages/cli-upload/test/unit/image-size.test.js b/packages/cli-upload/test/unit/image-size.test.js index 062da71e7..a8a7e5ffa 100644 --- a/packages/cli-upload/test/unit/image-size.test.js +++ b/packages/cli-upload/test/unit/image-size.test.js @@ -20,8 +20,9 @@ import { describe('unit / image-size', () => { let dirname, index = 0; - // these tests read real files — `imageSize` opens a descriptor and reads at - // offsets, which is the behaviour worth exercising against a real filesystem + // these tests read real files — `imageSize` opens a descriptor and reads a + // bounded prefix, which is the behaviour worth exercising against a real + // filesystem rather than a stubbed buffer beforeAll(() => { dirname = fs.mkdtempSync(path.join(os.tmpdir(), 'percy-image-size-')); }); @@ -46,6 +47,9 @@ describe('unit / image-size', () => { expect(imageSize(write(JPEG_200X150))).toEqual({ width: 200, height: 150 }); }); + // the underlying parser reads about ten formats, but `percy upload` accepts + // only png and jpeg, so anything else is rejected here even when it is a + // perfectly readable image it('returns null for other image formats', () => { expect(imageSize(write(GIF_PIXEL))).toBeNull(); }); diff --git a/yarn.lock b/yarn.lock index 86fe00efb..3932687b8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3830,7 +3830,7 @@ debounce@^1.2.0: resolved "https://registry.npmjs.org/debounce/-/debounce-1.2.0.tgz" integrity sha512-mYtLl1xfZLi1m4RtQYlZgJUNQjl4ZxVnHzIR8nLLgi4q1YT8o/WM+MK/f8yfcc9s5Ir5zRaPZyZU6xs1Syoocg== -debug@2.6.9: +debug@2, debug@2.6.9: version "2.6.9" resolved "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz" integrity sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA== @@ -3844,7 +3844,7 @@ debug@4, debug@^4.0.1, debug@^4.1.0, debug@^4.1.1, debug@^4.3.1, debug@^4.3.3, d dependencies: ms "^2.1.3" -debug@^3.2.7: +debug@^3.2.6, debug@^3.2.7: version "3.2.7" resolved "https://registry.npmjs.org/debug/-/debug-3.2.7.tgz" integrity sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ== @@ -5432,7 +5432,7 @@ humanize-ms@^1.2.1: dependencies: ms "^2.0.0" -iconv-lite@0.4.24, iconv-lite@^0.4.24: +iconv-lite@0.4.24, iconv-lite@^0.4.24, iconv-lite@^0.4.4: version "0.4.24" resolved "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.24.tgz" integrity sha512-v3MXnZAcvnywkTUEZomIActle7RXXeedOR31wwl7VlyoXO4Qi9arvSenNQWne1TcRwhCL1HwLI21bEqdpj8/rA== @@ -6868,6 +6868,15 @@ natural-compare@^1.4.0: resolved "https://registry.npmjs.org/natural-compare/-/natural-compare-1.4.0.tgz" integrity sha1-Sr6/7tdUHywnrPspvbvRXI1bpPc= +needle@^2.5.2: + version "2.9.1" + resolved "https://registry.yarnpkg.com/needle/-/needle-2.9.1.tgz#22d1dffbe3490c2b83e301f7709b6736cd8f2684" + integrity sha512-6R9fqJ5Zcmf+uYaFgdIHmLwNldn5HbK8L5ybn7Uz+ylX/rnOsSp1AHcvQSrCaFN+qNM1wpymHqD7mVasEOlHGQ== + dependencies: + debug "^3.2.6" + iconv-lite "^0.4.4" + sax "^1.2.4" + negotiator@0.6.2: version "0.6.2" resolved "https://registry.npmjs.org/negotiator/-/negotiator-0.6.2.tgz" @@ -7659,6 +7668,15 @@ pretty-format@^29.5.0: ansi-styles "^5.0.0" react-is "^18.0.0" +probe-image-size@^7.4.0: + version "7.4.0" + resolved "https://registry.yarnpkg.com/probe-image-size/-/probe-image-size-7.4.0.tgz#c189ae04e9aad1e3fa3a0998a956a7e9aab7f66d" + integrity sha512-cdEprVtZxV+awMde9X+4jILBFYh4CARxVrQaMl4wY4YcPWbul9jntXrIW95NInBDyJwcVUP3U0T6yukN8rMBaQ== + dependencies: + lodash.merge "^4.6.2" + needle "^2.5.2" + stream-parser "~0.3.1" + proc-log@^2.0.0, proc-log@^2.0.1: version "2.0.1" resolved "https://registry.npmjs.org/proc-log/-/proc-log-2.0.1.tgz" @@ -8137,6 +8155,11 @@ safe-regex-test@^1.0.3: resolved "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz" integrity sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg== +sax@^1.2.4: + version "1.6.1" + resolved "https://registry.yarnpkg.com/sax/-/sax-1.6.1.tgz#4c23cf608c0b693ab54b4b5888e92cfe977b9843" + integrity sha512-42tBVwLWnaQvW5zc4HbZrTuWccECCZfBi92FDuwtqxasH+JbPB3/FOKb1m222K42R4WxuxzzMsTswfzgtSu64Q== + "semver@2 || 3 || 4 || 5", semver@^5.6.0: version "5.7.1" resolved "https://registry.npmjs.org/semver/-/semver-5.7.1.tgz" @@ -8476,6 +8499,13 @@ statuses@~1.5.0: resolved "https://registry.npmjs.org/statuses/-/statuses-1.5.0.tgz" integrity sha1-Fhx9rBd2Wf2YEfQ3cfqZOBR4Yow= +stream-parser@~0.3.1: + version "0.3.1" + resolved "https://registry.yarnpkg.com/stream-parser/-/stream-parser-0.3.1.tgz#1618548694420021a1182ff0af1911c129761773" + integrity sha512-bJ/HgKq41nlKvlhccD5kaCr/P+Hu0wPNKPJOH7en+YrJu/9EgqUF+88w5Jb6KNcjOFMhfX4B2asfeAtIGuHObQ== + dependencies: + debug "2" + streamroller@^3.0.2: version "3.0.2" resolved "https://registry.npmjs.org/streamroller/-/streamroller-3.0.2.tgz" From 937dfae782952207f254f4c9917612e68654859c Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Mon, 17 Aug 2026 19:07:02 +0530 Subject: [PATCH 4/4] refactor(cli-upload): fold the dimension read into upload.js Drops the `image-size.js` adapter, the `fixtures.js` module and the `image-size` unit specs. Those existed to hold and prove a hand-rolled PNG/JPEG parser; with `probe-image-size` doing the parsing, they covered upstream's segment walking rather than anything this package owns. What remains of the adapter is a bounded header read and a format gate, both short enough to live beside their only caller. The four image fixtures the end-to-end specs use move inline, and a spec covers the format gate directly: a GIF named `.png` clears the extension filter and parses fine, so only the gate keeps it out. The change to `percy upload` is now the dependency swap plus the skip path. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli-upload/src/image-size.js | 46 ------ packages/cli-upload/src/upload.js | 41 ++++- packages/cli-upload/test/fixtures.js | 152 ------------------ .../cli-upload/test/unit/image-size.test.js | 119 -------------- packages/cli-upload/test/upload.test.js | 56 ++++++- 5 files changed, 92 insertions(+), 322 deletions(-) delete mode 100644 packages/cli-upload/src/image-size.js delete mode 100644 packages/cli-upload/test/fixtures.js delete mode 100644 packages/cli-upload/test/unit/image-size.test.js diff --git a/packages/cli-upload/src/image-size.js b/packages/cli-upload/src/image-size.js deleted file mode 100644 index e41fa99fd..000000000 --- a/packages/cli-upload/src/image-size.js +++ /dev/null @@ -1,46 +0,0 @@ -import fs from 'fs'; -// the `sync.js` entrypoint pulls in only the buffer parsers — none of the http -// or stream machinery, and so none of `needle`. Extension included because the -// package publishes no `exports` map, and ESM will not resolve it without one. -import probeSync from 'probe-image-size/sync.js'; - -// `percy upload` only ever accepts png, jpg and jpeg files (see ALLOWED_FILE_TYPES -// in upload.js), so anything else is rejected here even though the parser can read -// it. The previous dependency (`image-size`) is archived upstream and carries -// unfixable infinite-loop advisories (CVE-2025-71329, CVE-2025-71330) in parsers -// this command never wanted. `probe-image-size` has no ICNS, JXL or HEIF parser at -// all, and its ISOBMFF reader rejects a box smaller than its own header rather than -// advancing by it — the shape of every one of those advisories. -const SUPPORTED_TYPES = new Set(['png', 'jpg']); - -// A JPEG frame header sits behind however much metadata the encoder wrote, so the -// whole header cannot be read at a fixed offset. This is the limit `image-size` -// applied for the same reason, kept so that files it could read stay readable. -const MAX_HEADER_BYTES = 512 * 1024; - -// Reads the leading bytes of a file without pulling a multi-megabyte image into -// memory just to read its dimensions. -function readHeader(absolutePath) { - let fd = fs.openSync(absolutePath, 'r'); - - try { - let length = Math.min(fs.fstatSync(fd).size, MAX_HEADER_BYTES); - let buffer = Buffer.alloc(length); - // trailing bytes are only unwritten if the file shrank mid-read, but slicing - // to the actual count is correct either way and keeps this branchless - let bytesRead = fs.readSync(fd, buffer, 0, length, 0); - return buffer.subarray(0, bytesRead); - } finally { - fs.closeSync(fd); - } -} - -// Returns `{ width, height }` for a PNG or JPEG file, or null when the file is -// neither — including when its extension disagrees with its actual contents. -export function imageSize(absolutePath) { - let result = probeSync(readHeader(absolutePath)); - if (!result || !SUPPORTED_TYPES.has(result.type)) return null; - return { width: result.width, height: result.height }; -} - -export default imageSize; diff --git a/packages/cli-upload/src/upload.js b/packages/cli-upload/src/upload.js index d2fa6a65f..b1b3f710f 100644 --- a/packages/cli-upload/src/upload.js +++ b/packages/cli-upload/src/upload.js @@ -6,6 +6,29 @@ import * as UploadConfig from './config.js'; const ALLOWED_FILE_TYPES = /\.(png|jpg|jpeg)$/i; const ALLOWED_TOKEN_TYPES = ['web', 'generic']; +// The dimension reader recognises about ten formats; this command accepts two. +const SUPPORTED_IMAGE_TYPES = new Set(['png', 'jpg']); + +// A JPEG frame header sits behind however much metadata the encoder wrote, so +// dimensions cannot be read at a fixed offset. This is the limit the previous +// `image-size` dependency applied, kept so that no file it could read becomes +// unreadable. +const MAX_HEADER_BYTES = 512 * 1024; + +// Reads the leading bytes of a file, rather than pulling a multi-megabyte image +// into memory just to read its dimensions. +function readImageHeader(absolutePath) { + let fd = fs.openSync(absolutePath, 'r'); + + try { + let length = Math.min(fs.fstatSync(fd).size, MAX_HEADER_BYTES); + let header = Buffer.alloc(length); + return header.subarray(0, fs.readSync(fd, header, 0, length, 0)); + } finally { + fs.closeSync(fd); + } +} + // All BYOS screenshots have a fixed comparison tag export const BYOS_TAG = { name: 'Uploaded Screenshot', @@ -85,7 +108,10 @@ export const upload = command('upload', { exit(1, 'Invalid Token Type. Only "web" and "self-managed" token types are allowed.'); } - let { imageSize } = await import('./image-size.js'); + // `sync.js` is the buffer-parser entrypoint — none of the http or stream + // machinery. The extension is required because the package publishes no + // `exports` map and this one is ESM. + let { default: probeImageSize } = await import('probe-image-size/sync.js'); let { getImageResources } = await import('./utils.js'); // the internal discovery queue shares a concurrency with the snapshots queue @@ -97,14 +123,21 @@ export const upload = command('upload', { log.info(`Skipping unsupported file type: ${relativePath}`); } else { let absolutePath = path.resolve(args.dirname, relativePath); - let size = imageSize(absolutePath); + let probed = probeImageSize(readImageHeader(absolutePath)); - if (!size) { + // covers a file whose extension disagrees with its contents — including a + // readable image in a format this command does not accept + if (!probed || !SUPPORTED_IMAGE_TYPES.has(probed.type)) { log.info(`Skipping file with unreadable image data: ${relativePath}`); continue; } - let img = { relativePath, absolutePath, ...size }; + let img = { + relativePath, + absolutePath, + width: probed.width, + height: probed.height + }; let { dir, name, ext } = path.parse(relativePath); img.type = ext === '.png' ? 'png' : 'jpeg'; img.name = path.join(dir, name); diff --git a/packages/cli-upload/test/fixtures.js b/packages/cli-upload/test/fixtures.js deleted file mode 100644 index f7a16af90..000000000 --- a/packages/cli-upload/test/fixtures.js +++ /dev/null @@ -1,152 +0,0 @@ -// Real image bytes, kept as buffers rather than strings — the PNG signature -// starts with 0x89, which does not survive a round trip through UTF-8. - -const b64 = str => Buffer.from(str, 'base64'); - -// 1x1 red PNG -export const PNG_PIXEL = b64( - 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ' + - '/pLvAAAAAElFTkSuQmCC' -); - -// 120x80 red PNG -export const PNG_120X80 = b64( - 'iVBORw0KGgoAAAANSUhEUgAAAHgAAABQCAIAAABd+SbeAAAA4klEQVR4nO3OQQ0AIAADsfk3' + - 'DS7o40gqoDvb94AfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4' + - 'QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E' + - '+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQf' + - 'RPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRPhBhB9E+EGEH0T4QYQfRFwNVlys/6lZ' + - 'IQAAAABJRU5ErkJggg==' -); - -// 1x1 red JPEG -export const JPEG_PIXEL = b64( - '/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9' + - 'PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhC' + - 'Y2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAAR' + - 'CAABAAEDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA' + - 'AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK' + - 'FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG' + - 'h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl' + - '5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA' + - 'AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk' + - 'NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE' + - 'hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk' + - '5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwDFoooryz7w/9k=' -); - -// 200x150 red JPEG — its frame header sits past several kilobyte-scale -// quantization and Huffman tables, so reading it exercises segment walking -// rather than a fixed header offset. -export const JPEG_200X150 = b64( - '/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9' + - 'PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhC' + - 'Y2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAAR' + - 'CACWAMgDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA' + - 'AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK' + - 'FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG' + - 'h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl' + - '5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA' + - 'AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk' + - 'NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE' + - 'hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk' + - '5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwDFoooryz7wKKKKACiiigAooooAKKKKACii' + - 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + - 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + - 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + - 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + - 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + - 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + - 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA' + - 'KKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACii' + - 'igAooooAKKKKACiiigAooooAKKKKACiiigAooooAKKKKACiiigAooooA/9k=' -); - -// A GIF — accepted by neither the extension filter nor the size reader. -export const GIF_PIXEL = b64('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); - -// An ICNS buffer with valid magic bytes and a zero-valued entry length. -// This is the CVE-2025-71330 proof of concept: the archived `image-size` -// package looped forever on it because a zero-length entry never advanced the -// read offset. `percy upload` filters by extension, not magic bytes, so a file -// named `.png` reached that parser. -export function icnsZeroLengthEntry() { - let buffer = Buffer.alloc(64); - buffer.write('icns', 0, 'ascii'); - buffer.writeUInt32BE(64, 4); // file length - buffer.write('ic09', 8, 'ascii'); // first entry type - buffer.writeUInt32BE(0, 12); // first entry length - return buffer; -} - -// A JPEG whose first segment declares a length of zero. Advancing by a -// self-inclusive length below 2 would leave the read offset stationary. -export function jpegZeroLengthSegment() { - let buffer = Buffer.alloc(32); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffe0, 2); // APP0 - buffer.writeUInt16BE(0, 4); // segment length - return buffer; -} - -// A JPEG whose first segment length lands the walk on bytes that do not begin a -// marker — i.e. off the segment chain and into data. -export function jpegWalksIntoData() { - let buffer = Buffer.alloc(32); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffe0, 2); // APP0 - buffer.writeUInt16BE(4, 4); // length 4 ⇒ next marker expected at offset 8 - // offset 8 onward is left zeroed, so no 0xff marker prefix is found - return buffer; -} - -// A JPEG carrying a standalone marker (RST0, no length payload) ahead of the -// frame header. Mis-skipping it would desynchronise the walk. -export function jpegStandaloneMarkerBeforeFrame(width, height) { - // A SOF0 declaring one component is 11 bytes: length (2) + precision (1) + - // dimensions (4) + component count (1) + one component spec (3). The segment - // starts at offset 6, so the buffer has to reach offset 17 for the length it - // declares to be satisfiable — a parser that checks the declared length - // against the bytes actually present rejects anything shorter. - let buffer = Buffer.alloc(17); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffd0, 2); // RST0 — standalone - buffer.writeUInt16BE(0xffc0, 4); // SOF0 - buffer.writeUInt16BE(11, 6); // segment length - buffer.writeUInt8(8, 8); // sample precision - buffer.writeUInt16BE(height, 9); - buffer.writeUInt16BE(width, 11); - buffer.writeUInt8(1, 13); // component count - buffer.writeUInt8(1, 14); // component id - buffer.writeUInt8(0x11, 15); // sampling factors - buffer.writeUInt8(0, 16); // quantization table selector - return buffer; -} - -// A JPEG that reaches the start of scan without ever declaring a frame. -export function jpegScanWithoutFrame() { - let buffer = Buffer.alloc(32); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffda, 2); // SOS - buffer.writeUInt16BE(12, 4); // segment length - return buffer; -} - -// A JPEG that reaches end of image without ever declaring a frame. -export function jpegEndsWithoutFrame() { - let buffer = Buffer.alloc(32); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffd9, 2); // EOI - return buffer; -} - -// A JPEG that announces a frame header and then ends before it. Must stay at -// least 8 bytes long, or it is rejected at the signature read and never reaches -// the segment walk this is meant to exercise. -export function jpegTruncatedFrameHeader() { - let buffer = Buffer.alloc(8); - buffer.writeUInt16BE(0xffd8, 0); // SOI - buffer.writeUInt16BE(0xffc0, 2); // SOF0 - buffer.writeUInt16BE(11, 4); // length claims a payload the file does not have - return buffer; -} diff --git a/packages/cli-upload/test/unit/image-size.test.js b/packages/cli-upload/test/unit/image-size.test.js deleted file mode 100644 index a8a7e5ffa..000000000 --- a/packages/cli-upload/test/unit/image-size.test.js +++ /dev/null @@ -1,119 +0,0 @@ -import fs from 'fs'; -import os from 'os'; -import path from 'path'; -import { imageSize } from '../../src/image-size.js'; -import { - PNG_PIXEL, - PNG_120X80, - JPEG_PIXEL, - JPEG_200X150, - GIF_PIXEL, - icnsZeroLengthEntry, - jpegZeroLengthSegment, - jpegWalksIntoData, - jpegStandaloneMarkerBeforeFrame, - jpegScanWithoutFrame, - jpegEndsWithoutFrame, - jpegTruncatedFrameHeader -} from '../fixtures.js'; - -describe('unit / image-size', () => { - let dirname, index = 0; - - // these tests read real files — `imageSize` opens a descriptor and reads a - // bounded prefix, which is the behaviour worth exercising against a real - // filesystem rather than a stubbed buffer - beforeAll(() => { - dirname = fs.mkdtempSync(path.join(os.tmpdir(), 'percy-image-size-')); - }); - - afterAll(() => { - fs.rmSync(dirname, { recursive: true, force: true }); - }); - - let write = contents => { - let filename = path.join(dirname, `fixture-${index++}`); - fs.writeFileSync(filename, contents); - return filename; - }; - - it('reads PNG dimensions', () => { - expect(imageSize(write(PNG_PIXEL))).toEqual({ width: 1, height: 1 }); - expect(imageSize(write(PNG_120X80))).toEqual({ width: 120, height: 80 }); - }); - - it('reads JPEG dimensions', () => { - expect(imageSize(write(JPEG_PIXEL))).toEqual({ width: 1, height: 1 }); - expect(imageSize(write(JPEG_200X150))).toEqual({ width: 200, height: 150 }); - }); - - // the underlying parser reads about ten formats, but `percy upload` accepts - // only png and jpeg, so anything else is rejected here even when it is a - // perfectly readable image - it('returns null for other image formats', () => { - expect(imageSize(write(GIF_PIXEL))).toBeNull(); - }); - - it('returns null for files that are not images', () => { - expect(imageSize(write('not an image'))).toBeNull(); - expect(imageSize(write(Buffer.alloc(0)))).toBeNull(); - }); - - it('returns null for a truncated PNG', () => { - expect(imageSize(write(PNG_PIXEL.subarray(0, 16)))).toBeNull(); - }); - - it('returns null for a PNG whose first chunk is not IHDR', () => { - let png = Buffer.from(PNG_PIXEL); - png.write('IDAT', 12, 'ascii'); - expect(imageSize(write(png))).toBeNull(); - }); - - it('returns null for a JPEG too short to hold a signature', () => { - expect(imageSize(write(JPEG_PIXEL.subarray(0, 4)))).toBeNull(); - }); - - it('returns null when the segment chain leads into non-marker data', () => { - expect(imageSize(write(jpegWalksIntoData()))).toBeNull(); - }); - - it('returns null for a JPEG that starts its scan without a frame', () => { - expect(imageSize(write(jpegScanWithoutFrame()))).toBeNull(); - }); - - it('returns null for a JPEG that ends without a frame', () => { - expect(imageSize(write(jpegEndsWithoutFrame()))).toBeNull(); - }); - - it('returns null for a JPEG that ends before its frame header', () => { - expect(imageSize(write(jpegTruncatedFrameHeader()))).toBeNull(); - }); - - it('skips standalone markers to reach the frame header', () => { - expect(imageSize(write(jpegStandaloneMarkerBeforeFrame(320, 240)))) - .toEqual({ width: 320, height: 240 }); - }); - - // CVE-2025-71330 / CVE-2025-71329 — the advisories that made `image-size` - // unusable were all the same shape: a zero-valued length field left the read - // offset unchanged, so the parser looped forever and wedged the event loop. - it('terminates on an ICNS buffer with a zero-length entry', () => { - // named `.png` because `percy upload` filters on extension, which is how - // this buffer reached the ICNS parser in the first place - let filename = path.join(dirname, 'crafted.png'); - fs.writeFileSync(filename, icnsZeroLengthEntry()); - - expect(imageSize(filename)).toBeNull(); - }); - - it('terminates on a JPEG segment with a zero length', () => { - expect(imageSize(write(jpegZeroLengthSegment()))).toBeNull(); - }); - - it('terminates on a JPEG of nothing but 0xff fill bytes', () => { - let buffer = Buffer.alloc(4096, 0xff); - buffer.writeUInt16BE(0xffd8, 0); - - expect(imageSize(write(buffer))).toBeNull(); - }); -}); diff --git a/packages/cli-upload/test/upload.test.js b/packages/cli-upload/test/upload.test.js index 1089c92f3..fe31eaddf 100644 --- a/packages/cli-upload/test/upload.test.js +++ b/packages/cli-upload/test/upload.test.js @@ -1,7 +1,47 @@ import { fs, logger, api, setupTest } from '@percy/cli-command/test/helpers'; import upload from '@percy/cli-upload'; import { BYOS_TAG } from '../src/upload.js'; -import { PNG_PIXEL, JPEG_PIXEL, GIF_PIXEL, icnsZeroLengthEntry } from './fixtures.js'; + +// Real image bytes, kept as buffers rather than strings — the PNG signature +// starts with 0x89, which does not survive a round trip through UTF-8. +const b64 = str => Buffer.from(str, 'base64'); + +// 1x1 red PNG +const PNG_PIXEL = b64( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ' + + '/pLvAAAAAElFTkSuQmCC' +); + +// 1x1 red JPEG +const JPEG_PIXEL = b64( + '/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0oOjM9' + + 'PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/2wBDARESEhgVGC8aGi9jQjhC' + + 'Y2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2P/wAAR' + + 'CAABAAEDASIAAhEBAxEB/8QAHwAAAQUBAQEBAQEAAAAAAAAAAAECAwQFBgcICQoL/8QAtRAA' + + 'AgEDAwIEAwUFBAQAAAF9AQIDAAQRBRIhMUEGE1FhByJxFDKBkaEII0KxwRVS0fAkM2JyggkK' + + 'FhcYGRolJicoKSo0NTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqDhIWG' + + 'h4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1NXW19jZ2uHi4+Tl' + + '5ufo6erx8vP09fb3+Pn6/8QAHwEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoL/8QAtREA' + + 'AgECBAQDBAcFBAQAAQJ3AAECAxEEBSExBhJBUQdhcRMiMoEIFEKRobHBCSMzUvAVYnLRChYk' + + 'NOEl8RcYGRomJygpKjU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6goOE' + + 'hYaHiImKkpOUlZaXmJmaoqOkpaanqKmqsrO0tba3uLm6wsPExcbHyMnK0tPU1dbX2Nna4uPk' + + '5ebn6Onq8vP09fb3+Pn6/9oADAMBAAIRAxEAPwDFoooryz7w/9k=' +); + +// A GIF — readable by the parser, but not a format this command accepts. +const GIF_PIXEL = b64('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); + +// An ICNS buffer with valid magic bytes and a zero-valued entry length — the +// CVE-2025-71330 proof of concept. `image-size` looped forever on it because a +// zero-length entry never advanced the read offset. +const icnsZeroLengthEntry = () => { + let buffer = Buffer.alloc(64); + buffer.write('icns', 0, 'ascii'); + buffer.writeUInt32BE(64, 4); // file length + buffer.write('ic09', 8, 'ascii'); // first entry type + buffer.writeUInt32BE(0, 12); // first entry length + return buffer; +}; describe('percy upload', () => { beforeEach(async () => { @@ -186,6 +226,20 @@ describe('percy upload', () => { ])); }); + // the extension filter passes this through, and the parser reads GIFs happily, + // so only the format gate keeps it out + it('skips a readable image whose format is not png or jpeg', async () => { + fs.writeFileSync('images/gif-named.png', GIF_PIXEL); + await upload(['./images']); + + expect(logger.stderr).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Skipping file with unreadable image data: gif-named.png', + '[percy] Uploading 3 snapshots...', + '[percy] Finalized build #1: https://percy.io/test/test/123' + ])); + }); + it('does not upload snapshots and prints matching files with --dry-run', async () => { await upload(['./images', '--dry-run']);