Skip to content

Commit c4813f3

Browse files
CopilotHexagon
andauthored
fix: security, correctness, and performance improvements from TODO review (#103)
* fix: address H1-H3, M1-M4, M7 TODO items (security, correctness, perf) Agent-Logs-Url: https://github.com/cross-org/image/sessions/363016e9-df1d-4e62-b1e3-88d93ae1a755 Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com> * fix: CHANGELOG notation typo (200 M -> 200M) Agent-Logs-Url: https://github.com/cross-org/image/sessions/363016e9-df1d-4e62-b1e3-88d93ae1a755 Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com> * fix: apply deno fmt to CHANGELOG.md (CI formatting failure) Agent-Logs-Url: https://github.com/cross-org/image/sessions/917a56d1-bf05-42f8-84cc-96db29a01a88 Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
1 parent dc6918f commit c4813f3

File tree

8 files changed

+66
-31
lines changed

8 files changed

+66
-31
lines changed

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,25 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
2929
`src/utils/gif_decoder.ts` — prevents call-stack overflow
3030
(`RangeError: Maximum call stack
3131
size exceeded`) on adversarial GIF input
32+
- Security: PCX decoder now calls `validateImageDimensions` after computing width/height from the
33+
header — prevents excessively large unchecked allocations on crafted input
34+
- Correctness: TIFF multi-frame encoder (`encodeFrames`) now caches compressed frame data from the
35+
first pass and reuses it when writing `StripByteCounts` — previously the data was re-compressed
36+
independently, producing a byte-count that could differ from the actual strip data and making the
37+
TIFF unreadable
38+
- Correctness: ICO DIB decoder now uses `Math.floor` when computing `actualHeight` from the stored
39+
double-height field — previously an odd stored height (e.g. 5) produced a non-integer value (2.5)
40+
that caused `validateImageDimensions` to throw on valid ICO files
41+
- Correctness: PNG `decode()` loop now checks `pos + 8 > data.length` before reading each chunk
42+
header — prevents silent mis-decoding of truncated PNG files
43+
- Correctness: BMP `extractMetadata` now uses `readInt32LE` (signed) for the `xPelsPerMeter` /
44+
`yPelsPerMeter` DPI fields, consistent with `decode()` — previously a negative stored DPI was
45+
returned as a large positive value
46+
- Performance: JPEG IDCT now uses a precomputed 8×8 cosine table instead of calling `Math.cos` per
47+
coefficient — eliminates ~200M `Math.cos` calls when decoding a 2000×2000 JPEG
48+
- Performance: `medianFilter` now allocates the four channel-value arrays once outside the pixel
49+
loop instead of per-pixel — reduces GC pressure on large images
50+
- Fixed misleading comment in `adjustHue`: normalization produces 0–360, not −180 to 180
3251
- PNG decoder: 16-bit per-channel images (bitDepth=16) now decode correctly; the pixel stride was
3352
using a fixed 8-bit offset (`x*4`, `x*3`, `x`) causing pixel-offset corruption in 16-bit RGBA,
3453
RGB, and grayscale images

src/formats/bmp.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,8 @@ export class BMPFormat implements ImageFormat {
257257

258258
// DPI information (pixels per meter)
259259
if (headerSize >= 40) {
260-
const xPelsPerMeter = readUint32LE(data, 38);
261-
const yPelsPerMeter = readUint32LE(data, 42);
260+
const xPelsPerMeter = readInt32LE(data, 38);
261+
const yPelsPerMeter = readInt32LE(data, 42);
262262

263263
if (xPelsPerMeter > 0) {
264264
metadata.dpiX = Math.round(xPelsPerMeter * 0.0254);

src/formats/ico.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,11 @@ export class ICOFormat implements ImageFormat {
141141
const bitDepth = readUint16LE(data, 14);
142142
const compression = readUint32LE(data, 16);
143143

144-
// Validate dimensions
145-
validateImageDimensions(width, Math.abs(height) / 2); // DIB height includes both XOR and AND mask data
146-
147144
// ICO files store height as 2x actual height (for AND mask)
148-
const actualHeight = Math.abs(height) / 2;
145+
const actualHeight = Math.floor(Math.abs(height) / 2);
146+
147+
// Validate dimensions
148+
validateImageDimensions(width, actualHeight); // DIB height includes both XOR and AND mask data
149149

150150
// Only support uncompressed DIBs
151151
if (compression !== 0) {

src/formats/pcx.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { ImageData, ImageDecoderOptions, ImageFormat, ImageMetadata } from "../types.ts";
2+
import { validateImageDimensions } from "../utils/security.ts";
23

34
/**
45
* PCX format handler
@@ -53,6 +54,12 @@ export class PCXFormat implements ImageFormat {
5354
return Promise.reject(new Error("Invalid PCX dimensions"));
5455
}
5556

57+
try {
58+
validateImageDimensions(width, height);
59+
} catch (e) {
60+
return Promise.reject(e);
61+
}
62+
5663
// Decode RLE data
5764
let offset = 128;
5865
const scanlineLength = nPlanes * bytesPerLine;

src/formats/png.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export class PNGFormat extends PNGBase implements ImageFormat {
5757

5858
// Parse chunks
5959
while (pos < data.length) {
60+
if (pos + 8 > data.length) break;
6061
const length = this.readUint32(data, pos);
6162
pos += 4;
6263
const type = String.fromCharCode(

src/formats/tiff.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -573,8 +573,9 @@ export class TIFFFormat implements ImageFormat {
573573
let currentOffset = 8;
574574
const ifdOffsets: number[] = [];
575575
const pixelDataOffsets: number[] = [];
576+
const compressedFrames: Uint8Array[] = [];
576577

577-
// Write all pixel data first
578+
// Write all pixel data first, caching the compressed results
578579
for (const frame of imageData.frames) {
579580
pixelDataOffsets.push(currentOffset);
580581

@@ -590,6 +591,7 @@ export class TIFFFormat implements ImageFormat {
590591
pixelData = frame.data;
591592
}
592593

594+
compressedFrames.push(pixelData);
593595
for (let i = 0; i < pixelData.length; i++) {
594596
result.push(pixelData[i]);
595597
}
@@ -651,18 +653,8 @@ export class TIFFFormat implements ImageFormat {
651653
// RowsPerStrip
652654
this.writeIFDEntry(result, 0x0116, 4, 1, frame.height);
653655

654-
// StripByteCounts
655-
let pixelDataSize: number;
656-
if (compression === "lzw") {
657-
pixelDataSize = new TIFFLZWEncoder().compress(frame.data).length;
658-
} else if (compression === "packbits") {
659-
pixelDataSize = packBitsCompress(frame.data).length;
660-
} else if (compression === "deflate") {
661-
pixelDataSize = (await deflateCompress(frame.data)).length;
662-
} else {
663-
pixelDataSize = frame.data.length;
664-
}
665-
this.writeIFDEntry(result, 0x0117, 4, 1, pixelDataSize);
656+
// StripByteCounts — use the cached compressed length from the first pass
657+
this.writeIFDEntry(result, 0x0117, 4, 1, compressedFrames[i].length);
666658

667659
// XResolution
668660
const xResOffset = dataOffset;

src/utils/image_processing.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ function hslToRgb(h: number, s: number, l: number): [number, number, number] {
300300
*/
301301
export function adjustHue(data: Uint8Array, degrees: number): Uint8Array {
302302
const result = new Uint8Array(data.length);
303-
// Normalize rotation to -180 to 180 range
303+
// Normalize rotation to 0–360 range
304304
const rotation = ((degrees % 360) + 360) % 360;
305305

306306
for (let i = 0; i < data.length; i += 4) {
@@ -723,13 +723,17 @@ export function medianFilter(
723723
): Uint8Array {
724724
const result = new Uint8Array(data.length);
725725
const clampedRadius = Math.max(1, Math.floor(radius));
726+
const rValues: number[] = [];
727+
const gValues: number[] = [];
728+
const bValues: number[] = [];
729+
const aValues: number[] = [];
726730

727731
for (let y = 0; y < height; y++) {
728732
for (let x = 0; x < width; x++) {
729-
const rValues: number[] = [];
730-
const gValues: number[] = [];
731-
const bValues: number[] = [];
732-
const aValues: number[] = [];
733+
rValues.length = 0;
734+
gValues.length = 0;
735+
bValues.length = 0;
736+
aValues.length = 0;
733737

734738
// Collect values in kernel window
735739
for (let ky = -clampedRadius; ky <= clampedRadius; ky++) {

src/utils/jpeg_decoder.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,20 @@ interface JPEGDecoderOptions extends ImageDecoderOptions {
137137
extractCoefficients?: boolean;
138138
}
139139

140+
/**
141+
* Precomputed IDCT cosine table: IDCT_COS[k][n] = cos((2n+1)*k*PI/16)
142+
* Eliminates all Math.cos calls from the IDCT hot path.
143+
*/
144+
const IDCT_COS: Float64Array[] = Array.from(
145+
{ length: 8 },
146+
(_, k) =>
147+
Float64Array.from(
148+
{ length: 8 },
149+
(_, n) => Math.cos((2 * n + 1) * k * Math.PI / 16),
150+
),
151+
);
152+
const IDCT_SCALE = 1 / Math.sqrt(2);
153+
140154
export class JPEGDecoder {
141155
private data: Uint8Array;
142156
private pos: number = 0;
@@ -959,8 +973,7 @@ export class JPEGDecoder {
959973
}
960974

961975
private idct(block: number[] | Int32Array): void {
962-
// Simplified 2D IDCT
963-
// This is a basic implementation - not optimized
976+
// 2D IDCT using precomputed cosine table — no Math.cos in the hot path
964977
const temp = new Float32Array(64);
965978

966979
// 1D IDCT on rows
@@ -970,9 +983,8 @@ export class JPEGDecoder {
970983
for (let j = 0; j < 8; j++) {
971984
let sum = 0;
972985
for (let k = 0; k < 8; k++) {
973-
const c = k === 0 ? 1 / Math.sqrt(2) : 1;
974-
sum += c * block[offset + k] *
975-
Math.cos((2 * j + 1) * k * Math.PI / 16);
986+
const c = k === 0 ? IDCT_SCALE : 1;
987+
sum += c * block[offset + k] * IDCT_COS[k][j];
976988
}
977989
temp[offset + j] = sum / 2;
978990
}
@@ -983,8 +995,8 @@ export class JPEGDecoder {
983995
for (let i = 0; i < 8; i++) {
984996
let sum = 0;
985997
for (let k = 0; k < 8; k++) {
986-
const c = k === 0 ? 1 / Math.sqrt(2) : 1;
987-
sum += c * temp[k * 8 + j] * Math.cos((2 * i + 1) * k * Math.PI / 16);
998+
const c = k === 0 ? IDCT_SCALE : 1;
999+
sum += c * temp[k * 8 + j] * IDCT_COS[k][i];
9881000
}
9891001
// Level shift and clamp
9901002
block[i * 8 + j] = Math.max(

0 commit comments

Comments
 (0)