Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 15 additions & 18 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<type>' 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.

---

Expand Down
28 changes: 25 additions & 3 deletions src/formats/apng.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,32 @@ 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(
Comment on lines 127 to 132

Copilot AI Apr 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APNGFormat.decodeFrames first-pass chunk parsing does not validate that there are at least 8 bytes for length+type, nor that pos + length + 4 is within data.length before slicing chunk data and reading the stored CRC. With readUint32's out-of-range behavior, truncated/corrupt inputs can be mis-parsed and can produce misleading CRC errors (or be bypassed if CRC reads as 0). Add the same bounds checks used in extractMetadata before reading type/data/CRC, and throw a clear truncation/invalid chunk error.

Copilot uses AI. Check for mistakes.
data[pos],
data[pos + 1],
data[pos + 2],
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 });

Expand Down Expand Up @@ -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],
Expand All @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/formats/pam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
27 changes: 24 additions & 3 deletions src/formats/png.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,28 @@ 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],
data[pos + 2],
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;
Comment on lines 74 to +77

Copilot AI Apr 14, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In PNGFormat.decode's chunk loop there is no bounds check that pos + length + 4 is within data.length before slicing chunk data and reading the stored CRC. Because readUint32 treats out-of-range reads as zero, a truncated chunk can be misclassified as a CRC failure (and can even be bypassed if the computed CRC happens to be 0), and chunkData can be shorter than the declared length. Add an explicit bounds check (similar to extractMetadata's if (pos + length + 4 > data.length) ...) and throw a clear truncation/invalid chunk error before CRC verification.

Copilot uses AI. Check for mistakes.

// 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);
Expand Down Expand Up @@ -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],
Expand All @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions src/utils/gif_decoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 16 additions & 15 deletions src/utils/image_processing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 5 additions & 5 deletions test/utils/security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading