diff --git a/CHANGELOG.md b/CHANGELOG.md index 605738d..bb44f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). RGB, and grayscale images - PNG decoder: colorType 4 (grayscale+alpha) images are now supported instead of throwing an "Unsupported PNG color type: 4" error +- Correctness: PNG and APNG chunk CRC values are now verified during decode — crafted or corrupted + files that previously decoded silently with wrong pixel data now throw a descriptive error +- Robustness: GIF background-color index lookup now uses an explicit bounds check instead of `|| 0` + — prevents silently mapping an out-of-range palette index to black +- Performance: `adjustBrightness` clamp LUT is now computed once at module load time instead of on + every call — eliminates repeated 767-entry array allocation and initialisation - PNG decoder: sub-byte grayscale formats (bitDepth 1, 2, 4) now compute the correct scanline byte length (`ceil(width * bitsPerPixel / 8)`) and correctly unpack pixel values from packed bytes; previously the scanline was over-read and raw byte values were used directly as gray values diff --git a/TODO.md b/TODO.md index 3cc0e92..1683903 100644 --- a/TODO.md +++ b/TODO.md @@ -29,42 +29,39 @@ chunks, with only the small IFD section in a `number[]`. ### L1 — `src/formats/png.ts` — PNG chunk CRCs are never verified during decode -Chunk CRCs are silently skipped. A corrupt PNG file can decode with subtly wrong pixel data and no +~~Chunk CRCs are silently skipped. A corrupt PNG file can decode with subtly wrong pixel data and no error. The encode path correctly computes CRCs; parity with the decode path would improve -reliability. +reliability.~~ **Fixed:** CRC is now verified for every chunk in the PNG and APNG +decode/extractMetadata loops; corrupt or crafted files throw `"PNG chunk '' has invalid CRC"`. --- ### L2 — `src/formats/pam.ts:136-144` — PAM decoder silently rejects valid DEPTH values without documentation -The PAM format supports DEPTH 1 (grayscale), 2 (grayscale+alpha), and 3 (RGB). The implementation +~~The PAM format supports DEPTH 1 (grayscale), 2 (grayscale+alpha), and 3 (RGB). The implementation only handles DEPTH=4 (RGBA) and MAXVAL=255, throwing a generic error for everything else. This is an -intentional limitation but is not documented in the JSDoc, so callers receive a confusing error. - -**Fix:** Document the DEPTH/MAXVAL constraints in the JSDoc and consider a more descriptive error -message such as `"Only DEPTH=4 MAXVAL=255 PAM files are supported"`. +intentional limitation but is not documented in the JSDoc, so callers receive a confusing error.~~ +**Fixed:** The `decode()` JSDoc now explicitly documents the DEPTH 4 / MAXVAL 255 / TUPLTYPE +RGB_ALPHA constraint, and the error messages are descriptive. --- ### L3 — `src/utils/image_processing.ts:106-135` — `adjustBrightness` rebuilds the LUT on every call -The 767-entry brightness lookup table depends only on `amount`. It is re-created on every call. For -batch-processing many frames at the same brightness level this is unnecessary work. - -**Fix:** Accept the LUT as an optional pre-built parameter, or cache by `amount` with a simple -`Map`. +~~The 767-entry brightness lookup table depends only on `amount`. It is re-created on every call. +For batch-processing many frames at the same brightness level this is unnecessary work.~~ **Fixed:** +The clamp LUT is now a module-level `Uint8Array` constant computed once at load time; the per-call +code only derives the integer offset from `amount`. --- ### L4 — `src/utils/gif_decoder.ts:342-344` — Background color uses `|| 0` fallback instead of explicit bounds check -```ts -const bgR = colorTable[backgroundColorIndex * 3] || 0; -``` - -If `backgroundColorIndex * 3` is out of range, `colorTable[oob]` returns `undefined`, and +~~If `backgroundColorIndex * 3` is out of range, `colorTable[oob]` returns `undefined`, and `undefined || 0` silently returns 0 (black). This is functionally safe for most images but hides a -potential header-parsing error. An explicit bounds check and warning would aid debugging. +potential header-parsing error. An explicit bounds check and warning would aid debugging.~~ +**Fixed:** Replaced the three `|| 0` lookups with a single `bgOffset + 2 < colorTable.length` bounds +check. --- diff --git a/src/formats/apng.ts b/src/formats/apng.ts index ace10dc..844c888 100644 --- a/src/formats/apng.ts +++ b/src/formats/apng.ts @@ -125,8 +125,10 @@ export class APNGFormat extends PNGBase implements ImageFormat { }> = []; while (pos < data.length) { + if (pos + 8 > data.length) break; const length = this.readUint32(data, pos); pos += 4; + const typePos = pos; const type = String.fromCharCode( data[pos], data[pos + 1], @@ -134,10 +136,21 @@ export class APNGFormat extends PNGBase implements ImageFormat { data[pos + 3], ); pos += 4; + + if (length > data.length - pos - 4) break; + const chunkData = data.slice(pos, pos + length); const chunkPos = pos; pos += length; - pos += 4; // Skip CRC + const storedCrc = this.readUint32(data, pos); + pos += 4; + + // Verify CRC (covers chunk type + chunk data) + if ( + storedCrc !== this.crc32(data.subarray(typePos, typePos + 4 + length)) + ) { + throw new Error(`PNG chunk '${type}' has invalid CRC`); + } chunkList.push({ type, data: chunkData, pos: chunkPos }); @@ -522,6 +535,7 @@ export class APNGFormat extends PNGBase implements ImageFormat { const length = this.readUint32(data, pos); pos += 4; + const typePos = pos; const type = String.fromCharCode( data[pos], data[pos + 1], @@ -530,11 +544,19 @@ export class APNGFormat extends PNGBase implements ImageFormat { ); pos += 4; - if (pos + length + 4 > data.length) break; + if (length > data.length - pos - 4) break; const chunkData = data.slice(pos, pos + length); pos += length; - pos += 4; // Skip CRC + const storedCrc = this.readUint32(data, pos); + pos += 4; + + // Verify CRC (covers chunk type + chunk data) + if ( + storedCrc !== this.crc32(data.subarray(typePos, typePos + 4 + length)) + ) { + throw new Error(`PNG chunk '${type}' has invalid CRC`); + } if (type === "IHDR") { width = this.readUint32(chunkData, 0); diff --git a/src/formats/pam.ts b/src/formats/pam.ts index 1d13bf1..08f1677 100644 --- a/src/formats/pam.ts +++ b/src/formats/pam.ts @@ -43,6 +43,11 @@ export class PAMFormat implements ImageFormat { /** * Decode PAM image data to RGBA + * + * **Supported subset:** only PAM files with `DEPTH 4`, `MAXVAL 255`, and + * `TUPLTYPE RGB_ALPHA` are supported. Files with other DEPTH or MAXVAL values + * will throw a descriptive error. + * * @param data Raw PAM image data * @returns Decoded image data with RGBA pixels */ diff --git a/src/formats/png.ts b/src/formats/png.ts index f10c0c5..c12851a 100644 --- a/src/formats/png.ts +++ b/src/formats/png.ts @@ -60,6 +60,7 @@ export class PNGFormat extends PNGBase implements ImageFormat { if (pos + 8 > data.length) break; const length = this.readUint32(data, pos); pos += 4; + const typePos = pos; const type = String.fromCharCode( data[pos], data[pos + 1], @@ -67,9 +68,20 @@ export class PNGFormat extends PNGBase implements ImageFormat { data[pos + 3], ); pos += 4; + + if (length > data.length - pos - 4) break; + const chunkData = data.slice(pos, pos + length); pos += length; - pos += 4; // Skip CRC + const storedCrc = this.readUint32(data, pos); + pos += 4; + + // Verify CRC (covers chunk type + chunk data) + if ( + storedCrc !== this.crc32(data.subarray(typePos, typePos + 4 + length)) + ) { + throw new Error(`PNG chunk '${type}' has invalid CRC`); + } if (type === "IHDR") { width = this.readUint32(chunkData, 0); @@ -217,6 +229,7 @@ export class PNGFormat extends PNGBase implements ImageFormat { const length = this.readUint32(data, pos); pos += 4; + const typePos = pos; const type = String.fromCharCode( data[pos], data[pos + 1], @@ -225,11 +238,19 @@ export class PNGFormat extends PNGBase implements ImageFormat { ); pos += 4; - if (pos + length + 4 > data.length) break; + if (length > data.length - pos - 4) break; const chunkData = data.slice(pos, pos + length); pos += length; - pos += 4; // Skip CRC + const storedCrc = this.readUint32(data, pos); + pos += 4; + + // Verify CRC (covers chunk type + chunk data) + if ( + storedCrc !== this.crc32(data.subarray(typePos, typePos + 4 + length)) + ) { + throw new Error(`PNG chunk '${type}' has invalid CRC`); + } if (type === "IHDR") { width = this.readUint32(chunkData, 0); diff --git a/src/utils/gif_decoder.ts b/src/utils/gif_decoder.ts index b2fff96..c662355 100644 --- a/src/utils/gif_decoder.ts +++ b/src/utils/gif_decoder.ts @@ -339,9 +339,10 @@ export class GIFDecoder { const rgba = new Uint8Array(canvasWidth * canvasHeight * 4); // Fill with background color - const bgR = colorTable[backgroundColorIndex * 3] || 0; - const bgG = colorTable[backgroundColorIndex * 3 + 1] || 0; - const bgB = colorTable[backgroundColorIndex * 3 + 2] || 0; + const bgOffset = backgroundColorIndex * 3; + const bgR = bgOffset + 2 < colorTable.length ? colorTable[bgOffset] : 0; + const bgG = bgOffset + 2 < colorTable.length ? colorTable[bgOffset + 1] : 0; + const bgB = bgOffset + 2 < colorTable.length ? colorTable[bgOffset + 2] : 0; for (let i = 0; i < rgba.length; i += 4) { rgba[i] = bgR; diff --git a/src/utils/image_processing.ts b/src/utils/image_processing.ts index 9b355dc..c8be6f9 100644 --- a/src/utils/image_processing.ts +++ b/src/utils/image_processing.ts @@ -18,6 +18,17 @@ function isLittleEndian(): boolean { // Cache the endianness check result const IS_LITTLE_ENDIAN = isLittleEndian(); +// Pre-computed clamp lookup table for adjustBrightness. +// Covers the range [-255, 511] offset by 255 so the index is always non-negative. +// Index: (pixelValue + adjustInt + 255), result: clamped value in [0, 255]. +const BRIGHTNESS_LUT_SIZE = 767; +const BRIGHTNESS_LUT_OFFSET = 255; +const _BRIGHTNESS_LUT = new Uint8Array(BRIGHTNESS_LUT_SIZE); +for (let _i = 0; _i < BRIGHTNESS_LUT_SIZE; _i++) { + const _v = _i - BRIGHTNESS_LUT_OFFSET; + _BRIGHTNESS_LUT[_i] = _v < 0 ? 0 : (_v > 255 ? 255 : _v); +} + /** * Composite one image on top of another at a specified position * @param base Base image data (RGBA) @@ -111,23 +122,13 @@ export function adjustBrightness( const clampedAmount = Math.max(-1, Math.min(1, amount)); const adjust = clampedAmount * 255; - // Pre-compute lookup table for clamping - // Range: -255 to 511 (data value 0-255 + adjust -255 to 255), offset by 255 for zero-based index - const LUT_SIZE = 767; - const LUT_OFFSET = 255; - const lut = new Uint8Array(LUT_SIZE); - for (let i = 0; i < LUT_SIZE; i++) { - const value = i - LUT_OFFSET; - lut[i] = value < 0 ? 0 : (value > 255 ? 255 : value); - } - - // Use bitwise OR for fast rounding (equivalent to Math.round for positive numbers) - const adjustInt = (adjust + 0.5) | 0; + // Use symmetric rounding so negative and positive brightness adjustments behave consistently + const adjustInt = Math.round(adjust); for (let i = 0; i < data.length; i += 4) { - result[i] = lut[data[i] + adjustInt + LUT_OFFSET]; // R - result[i + 1] = lut[data[i + 1] + adjustInt + LUT_OFFSET]; // G - result[i + 2] = lut[data[i + 2] + adjustInt + LUT_OFFSET]; // B + result[i] = _BRIGHTNESS_LUT[data[i] + adjustInt + BRIGHTNESS_LUT_OFFSET]; // R + result[i + 1] = _BRIGHTNESS_LUT[data[i + 1] + adjustInt + BRIGHTNESS_LUT_OFFSET]; // G + result[i + 2] = _BRIGHTNESS_LUT[data[i + 2] + adjustInt + BRIGHTNESS_LUT_OFFSET]; // B result[i + 3] = data[i + 3]; // A } diff --git a/test/utils/security.test.ts b/test/utils/security.test.ts index 788bb19..6cfd50c 100644 --- a/test/utils/security.test.ts +++ b/test/utils/security.test.ts @@ -159,11 +159,11 @@ test("Security - PNG decoder rejects oversized images", async () => { 0, // Compression: 0 0, // Filter: 0 0, // Interlace: 0 - // CRC (not validated in this test) - 0, - 0, - 0, - 0, + // CRC for type "IHDR" + data above + 227, + 230, + 167, + 180, ]); await assertRejects(