From fc5a1143bbdfc071a080d9d5ed635122a406494f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Afonso=20Janu=C3=A1rio?= Date: Fri, 4 Sep 2026 20:23:35 +0100 Subject: [PATCH] Turn a corrupt PNG's decode crash into a catchable error png-js decodes IDAT pixel data through Node's async zlib.inflate and rethrows any inflate error from inside that callback. That happens on a later tick, so it can't be caught by a try/catch around document.image(), and it takes the whole process down (issue #1747). Added an inflateSync pass on the same compressed bytes right before handing them to png-js's decodePixels, in all three places pdfkit calls it (splitAlphaChannel, loadIndexedAlphaChannel, decodeData). Since it runs synchronously inside embed(), which is itself called synchronously from document.image(), a bad IDAT stream now throws in the same spot PDFImage.open() already throws for other malformed input, instead of crashing later from a spot nothing can catch. The #zlib abstraction only had deflateSync before this (used for re-encoding decoded pixels), so extended both implementations with an inflateSync as well: Node's zlib.inflateSync for the node build, and fflate's unzlibSync for the browser build, since that's the inverse of the zlibSync it already uses for deflateSync. Added a regression test reproducing the exact corrupted PNG from the issue and confirming document.image() now throws synchronously. Confirmed it fails on the unmodified code (as an uncaught exception, matching the reported crash) and passes with the fix. Also added direct round-trip and corrupt-data tests for the new inflateSync on both zlib implementations. Full suite (unit + visual, 494 tests) and eslint both pass. --- lib/image/png.js | 17 ++++++++++++++--- lib/zlib/browser.js | 3 ++- lib/zlib/node.js | 1 + tests/unit/image.spec.js | 29 +++++++++++++++++++++++++++++ tests/unit/zlib.spec.js | 16 ++++++++++++++++ 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/lib/image/png.js b/lib/image/png.js index f3b405c3..b88e77a6 100644 --- a/lib/image/png.js +++ b/lib/image/png.js @@ -121,8 +121,19 @@ class PNGImage { return (this.imgData = null); } + // png-js decodes pixel data through Node's async zlib.inflate and rethrows + // any inflate error from inside that callback, which happens on a later + // tick and so can't be caught by a try/catch around doc.image(). Running + // an inflateSync pass on the same bytes first surfaces the same failure + // synchronously, in a spot the caller can actually catch, the same way + // PDFImage.open() already throws synchronously for other bad input. + decodePixels(callback) { + zlib.inflateSync(this.imgData); + return this.image.decodePixels(callback); + } + splitAlphaChannel() { - return this.image.decodePixels((pixels) => { + return this.decodePixels((pixels) => { let a, p; const colorCount = this.image.colors; const pixelCount = this.width * this.height; @@ -151,7 +162,7 @@ class PNGImage { loadIndexedAlphaChannel() { const transparency = this.image.transparency.indexed; const isInterlaced = this.image.interlaceMethod === 1; - return this.image.decodePixels((pixels) => { + return this.decodePixels((pixels) => { const alphaChannel = new Uint8Array(this.width * this.height); let i = 0; @@ -170,7 +181,7 @@ class PNGImage { } decodeData() { - this.image.decodePixels((pixels) => { + this.decodePixels((pixels) => { this.imgData = zlib.deflateSync(pixels); this.finalize(); }); diff --git a/lib/zlib/browser.js b/lib/zlib/browser.js index bb93cbb5..b9294a0b 100644 --- a/lib/zlib/browser.js +++ b/lib/zlib/browser.js @@ -1,5 +1,6 @@ -import { zlibSync } from 'fflate'; +import { zlibSync, unzlibSync } from 'fflate'; export default { deflateSync: (data) => zlibSync(data), + inflateSync: (data) => unzlibSync(data), }; diff --git a/lib/zlib/node.js b/lib/zlib/node.js index 2b131928..579e2590 100644 --- a/lib/zlib/node.js +++ b/lib/zlib/node.js @@ -2,4 +2,5 @@ import zlib from 'zlib'; export default { deflateSync: (data) => zlib.deflateSync(data), + inflateSync: (data) => zlib.inflateSync(data), }; diff --git a/tests/unit/image.spec.js b/tests/unit/image.spec.js index 680d7752..304f29f7 100644 --- a/tests/unit/image.spec.js +++ b/tests/unit/image.spec.js @@ -43,6 +43,35 @@ describe('Image', function () { expect(jpeg.colorSpace).toBe('DeviceRGB'); }); + test('a PNG with a corrupted IDAT chunk throws synchronously instead of crashing later (issue #1747)', () => { + const chunk = (type, data) => { + const len = Buffer.alloc(4); + len.writeUInt32BE(data.length); + const body = Buffer.concat([Buffer.from(type, 'ascii'), data]); + const crc = Buffer.alloc(4); + return Buffer.concat([len, body, crc]); + }; + + const ihdr = Buffer.alloc(13); + ihdr.writeUInt32BE(1, 0); // width + ihdr.writeUInt32BE(1, 4); // height + ihdr[8] = 8; // bit depth + ihdr[9] = 6; // color type: RGBA, forces the decodePixels path + + const corruptPng = Buffer.concat([ + Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), + chunk('IHDR', ihdr), + chunk('IDAT', Buffer.from([0xde, 0xad, 0xbe, 0xef])), // not valid deflate data + chunk('IEND', Buffer.alloc(0)), + ]); + + // The bug: embed() used to hand this straight to png-js's async + // decodePixels, which rethrows inflate errors from inside a zlib + // callback on a later tick, so this document.image() call would crash + // the process instead of throwing something this test could catch. + expect(() => document.image(corruptPng, 0, 0)).toThrow(); + }); + describe('opacity', function () { test('adds an ExtGState with the correct ca value', () => { document.image('./tests/images/bee.png', 0, 0, { opacity: 0.5 }); diff --git a/tests/unit/zlib.spec.js b/tests/unit/zlib.spec.js index d7f5e3ea..bbca836d 100644 --- a/tests/unit/zlib.spec.js +++ b/tests/unit/zlib.spec.js @@ -15,4 +15,20 @@ describe('zlib', () => { expect(compressed.length).toBeLessThan(data.length); expect(Buffer.from(zlib.inflateSync(compressed))).toEqual(data); }); + + test.each([ + ['node', nodeZlib], + ['browser', browserZlib], + ])('%s inflateSync reverses deflateSync', (_, impl) => { + const compressed = impl.deflateSync(data); + expect(Buffer.from(impl.inflateSync(compressed))).toEqual(data); + }); + + test.each([ + ['node', nodeZlib], + ['browser', browserZlib], + ])('%s inflateSync throws on data that was never validly compressed', (_, impl) => { + const garbage = Buffer.from([0xde, 0xad, 0xbe, 0xef]); + expect(() => impl.inflateSync(garbage)).toThrow(); + }); });