fix(cli-upload): replace image-size with probe-image-size (PER-10427)

packages/cli-upload/package.json

@@ -35,6 +35,6 @@
   "dependencies": {
     "@percy/cli-command": "1.32.7-beta.0",
     "fast-glob": "^3.2.11",
-    "image-size": "~1.0.2"
+    "probe-image-size": "^7.4.0"
   }
 }

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 { default: imageSize } = await import('image-size');
+  // `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,7 +123,21 @@ 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 probed = probeImageSize(readImageHeader(absolutePath));
+
+      // 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,
+        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);

packages/cli-upload/test/upload.test.js

@@ -2,10 +2,46 @@ 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();
+// 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 () => {
@@ -15,13 +51,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 +198,48 @@ 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'
+    ]));
+  });
+
+  // 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']);
 

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==
@@ -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"
@@ -6875,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"
@@ -7666,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"
@@ -7778,13 +7789,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"
@@ -8151,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"
@@ -8490,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"