Skip to content

Commit 85e19ce

Browse files
matmenclaude
andcommitted
Fix ReDoS in the SVG parser, clean up ESLint and SonarJS findings
ESLint (typescript-eslint strict type-checked) with SonarJS, knip and a regex timing harness over every regular expression of src: - svg.ts: regular expressions that backtracked on hostile input are replaced: the stylesheet (blocks and comments) is scanned with indexOf, url() references by a small scanner, numbers use a pattern whose alternatives can't match the same digits twice, colors and lengths are trimmed instead of matching spaces twice. A 200,000 character style without braces took 60 s, url(#a followed by spaces was exponential; all of them take a few ms now (regression tests). The 1952 sample SVGs render the same; an unclosed style rule at the end applies now, like in browsers - types: the TIFF tags, quantizer lookup and SVG arrays are typed (no any), Shaper hooks are function properties (no unbound methods), redundant assertions removed - layout.ts: one coverage list helper for contexts and reverse chaining; text.ts, buffer.ts, machine.ts: unused helpers removed - one statement per line where single-line blocks read like bugs (vp8.ts, hinting.ts), substr -> slice, prefer-const The remaining findings are style (cognitive complexity, nested ternaries, const enum comparisons) or defensive checks TypeScript considers impossible (index lookups, input from JavaScript callers). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJgRK5qpwJEbHPuFv6kgAQ
1 parent a6c9ab2 commit 85e19ce

18 files changed

Lines changed: 179 additions & 68 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@
7070

7171
### Fixes
7272

73+
- `Image.renderSVG`: SVGs with long malformed styles, `url()` references, lengths or colors (e.g. a stylesheet of
74+
200,000 characters without braces, which took a minute) parse in linear time instead of backtracking quadratically
75+
or worse in regular expressions. A style rule that isn't closed at the end of the stylesheet applies now, as in
76+
browsers.
7377
- NaN and infinite arguments throw a RangeError naming the argument instead of silently drawing black, transparent
7478
or nothing: colors (`fill`, `setPixelAt`, `drawBox`, `drawCircle`, `trim`, `colorToAlpha`, `Image.gradient`, the
7579
text and stroke colors, `Image.rgbaToColor` and the other color conversions, so `Image.rgbToColor(NaN, 0, 0)` is no

‎src/codecs/png.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ function hexProfile(text: string): Uint8Array {
729729
if (!match) return new Uint8Array(0);
730730
const digits = match[2].replace(/\s+/g, '');
731731
const out = new Uint8Array(Math.min(+match[1], digits.length >> 1));
732-
for (let i = 0; i < out.length; i++) out[i] = parseInt(digits.substr(i * 2, 2), 16);
732+
for (let i = 0; i < out.length; i++) out[i] = parseInt(digits.slice(i * 2, i * 2 + 2), 16);
733733
return out;
734734
}
735735

‎src/codecs/quantize.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export function quantize(pixels: Uint8Array, width: number, { maxColors = 256, q
9898

9999
// returns an exact palette if the opaque pixels use at most `limit` distinct colors
100100
function exactPalette(pixels: Uint8Array, limit: number): { palette: Uint8Array, indices: Uint8Array } | null {
101-
const lookup = new Map();
101+
const lookup = new Map<number, number>();
102102
const indices = new Uint8Array(pixels.length >> 2);
103103
const palette: number[] = [];
104104

‎src/codecs/tiff.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export async function decode(data: Uint8Array, maxPixels = MAX_PIXELS): Promise<
7171
if (ifdOffset + 2 > data.length) throw new Error('TIFF: invalid IFD offset');
7272

7373
// read the first IFD
74-
const tags = new Map();
74+
const tags = new Map<number, { type: number, count: number, offset: number }>();
7575
const entryCount = big ? u64(ifdOffset) : u16(ifdOffset);
7676
const entrySize = big ? 20 : 12;
7777
const inlineSize = big ? 8 : 4;
@@ -188,7 +188,8 @@ export async function decode(data: Uint8Array, maxPixels = MAX_PIXELS): Promise<
188188

189189
// JPEG compressed segments decode straight to RGBA
190190
if (compression === 7) {
191-
const tables = tags.has(TAGS.JPEGTables) ? data.subarray(tags.get(TAGS.JPEGTables).offset, tags.get(TAGS.JPEGTables).offset + tags.get(TAGS.JPEGTables).count) : null;
191+
const jpegTables = tags.get(TAGS.JPEGTables);
192+
const tables = jpegTables ? data.subarray(jpegTables.offset, jpegTables.offset + jpegTables.count) : null;
192193
const transform = photometric === 6 ? 'ycc' : photometric === 2 ? 'rgb' : photometric === 5 ? 'cmyk' : null;
193194

194195
for (let s = 0; s < segmentsPerPlane; s++) {

‎src/codecs/vp8.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,15 @@ function predictBlock(mode: number, p: Uint8Array, base: number, stride: number,
474474
if (mode === DC_PRED) {
475475
const shift = size === 16 ? 3 : 2;
476476
let sum = 0, count = 0;
477-
if (mby > 0) { for (let i = 0; i < size; i++) sum += p[above + i]; count++; }
478-
if (mbx > 0) { for (let i = 0; i < size; i++) sum += p[base + i * stride - 1]; count++; }
477+
// (the edges above and left that exist)
478+
if (mby > 0) {
479+
for (let i = 0; i < size; i++) sum += p[above + i];
480+
count++;
481+
}
482+
if (mbx > 0) {
483+
for (let i = 0; i < size; i++) sum += p[base + i * stride - 1];
484+
count++;
485+
}
479486
const v = count === 0 ? 128 : sum + (1 << shift + count - 1) >> shift + count;
480487
for (let y = 0; y < size; y++) p.fill(v, base + y * stride, base + y * stride + size);
481488
} else if (mode === V_PRED) {

‎src/codecs/zlib.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ function inflate(dat: Uint8Array, buf: Uint8Array): Uint8Array {
110110
let final = 0, pos = 0, bt = 0;
111111

112112
do {
113-
let lm: Uint16Array, dm: Uint16Array | null = null, lbt: number, dbt = 0;
113+
let lm: Uint16Array, dm: Uint16Array, lbt: number, dbt: number;
114114
// a block's header (every block is decoded to its end before the next one)
115115
final = bits(dat, pos, 1);
116116
const type = bits(dat, pos + 1, 3);
@@ -191,7 +191,7 @@ function inflate(dat: Uint8Array, buf: Uint8Array): Uint8Array {
191191
pos += b;
192192
}
193193

194-
const d = (dm as Uint16Array)[bits16(dat, pos) & dms], dsym = d >> 4;
194+
const d = dm[bits16(dat, pos) & dms], dsym = d >> 4;
195195
if (!d) throw new Error('zlib: invalid distance');
196196
pos += d & 15;
197197

‎src/image.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,10 +1749,10 @@ export class Image {
17491749
const gradients: Array<{ min: number, max: number, gradient: (position: number) => Color }> = [];
17501750

17511751
for (let i = 0; i < positions.length; i++) {
1752-
let minPos = positions[i - 1];
1752+
const minPos = positions[i - 1];
17531753
if (minPos === undefined) continue;
17541754

1755-
let maxPos = positions[i];
1755+
const maxPos = positions[i];
17561756

17571757
let minVal = values[i - 1];
17581758
if (minVal === undefined) minVal = values[i];

‎src/render/buffer.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ export const enum GlyphProps {
99
BASE = 0x02,
1010
LIGATURE = 0x04,
1111
MARK = 0x08,
12-
CLASS_MASK = BASE | LIGATURE | MARK,
1312
SUBSTITUTED = 0x10,
1413
LIGATED = 0x20,
1514
MULTIPLIED = 0x40,
@@ -384,8 +383,6 @@ export const isContinuation = (info: GlyphInfo): boolean => (info.unicodeProps &
384383
const isFormat = (info: GlyphInfo): boolean => generalCategory(info) === Category.Cf;
385384
export const isZWNJ = (info: GlyphInfo): boolean => isFormat(info) && (info.unicodeProps & UnicodeProps.ZWNJ) !== 0;
386385
export const isZWJ = (info: GlyphInfo): boolean => isFormat(info) && (info.unicodeProps & UnicodeProps.ZWJ) !== 0;
387-
export const isJoiner = (info: GlyphInfo): boolean => isFormat(info) && (info.unicodeProps & (UnicodeProps.ZWJ | UnicodeProps.ZWNJ)) !== 0;
388-
export const isVariationSelector = (info: GlyphInfo): boolean => isFormat(info) && (info.unicodeProps & UnicodeProps.VS) !== 0;
389386

390387
export const IS_LIG_BASE = 0x10;
391388
export const ligId = (info: GlyphInfo): number => info.ligProps >> 5;

‎src/render/hinting.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,16 @@ class Execution {
777777
break;
778778
case 0x59: break; // EIF
779779
case 0x1c: next = ip + this.pop(); break; // JMPR
780-
case 0x78: { const e = this.pop(), offset = this.pop(); if (e) next = ip + offset; break; } // JROT
781-
case 0x79: { const e = this.pop(), offset = this.pop(); if (!e) next = ip + offset; break; } // JROF
780+
case 0x78: { // JROT
781+
const e = this.pop(), offset = this.pop();
782+
if (e) next = ip + offset;
783+
break;
784+
}
785+
case 0x79: { // JROF
786+
const e = this.pop(), offset = this.pop();
787+
if (!e) next = ip + offset;
788+
break;
789+
}
782790
case 0x2c: case 0x89: { // FDEF, IDEF
783791
const n = this.pop();
784792
let p = next;
@@ -863,8 +871,12 @@ class Execution {
863871
const mv = this.moveVector();
864872
const dx = d * mv.x, dy = d * mv.y;
865873
const shift = (z: Zone, p: number, touch: boolean): void => {
866-
z.curX[p] += dx; z.curY[p] += dy;
867-
if (touch) { if (gs.fv.x !== 0) z.touchX[p] = 1; if (gs.fv.y !== 0) z.touchY[p] = 1; }
874+
z.curX[p] += dx;
875+
z.curY[p] += dy;
876+
if (touch) {
877+
if (gs.fv.x !== 0) z.touchX[p] = 1;
878+
if (gs.fv.y !== 0) z.touchY[p] = 1;
879+
}
868880
};
869881

870882
if (op <= 0x33) { // SHP

‎src/render/layout.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -743,13 +743,7 @@ export class LayoutTable {
743743
}
744744

745745
if (format === 3) {
746-
// coverages as offsets, matched by index into a list of tests
747-
const tests: Array<(glyph: number) => boolean> = [];
748-
const coverages = (o: number, count: number): number[] => Array.from({ length: count }, (_, j) => {
749-
tests.push(covered(f, offset + f.u16(o + j * 2)));
750-
return tests.length - 1;
751-
});
752-
const matchCoverage: MatchFunc = (info, value) => tests[value](info.codepoint);
746+
const { coverages, matchCoverage, tests } = coverageList(f, offset);
753747
let r: Rule, first: number;
754748
if (!chained) {
755749
const inputCount = f.u16(offset + 2), recordCount = f.u16(offset + 4);
@@ -890,18 +884,13 @@ export class LayoutTable {
890884
// reverse chaining single
891885
case 8: {
892886
const backtrackCount = f.u16(offset + 4);
893-
const tests: Array<(glyph: number) => boolean> = [];
894-
const list = (o: number, count: number): number[] => Array.from({ length: count }, (_, j) => {
895-
tests.push(covered(f, offset + f.u16(o + j * 2)));
896-
return tests.length - 1;
897-
});
887+
const { coverages: list, matchCoverage } = coverageList(f, offset);
898888
const backtrack = list(offset + 6, backtrackCount);
899889
let p = offset + 6 + backtrackCount * 2;
900890
const lookaheadCount = f.u16(p);
901891
const lookahead = list(p + 2, lookaheadCount);
902892
p += 2 + lookaheadCount * 2;
903893
const substituteCount = f.u16(p), substitutes = p + 2;
904-
const matchCoverage: MatchFunc = (info, value) => tests[value](info.codepoint);
905894
return { coverage: has, would: single, apply: c => {
906895
const index = coverage(c.buffer.cur().codepoint);
907896
if (index < 0) return false;
@@ -1176,6 +1165,17 @@ export function applySynthetic(buffer: Buffer, gdef: GDEF, l: Lookup, mask: numb
11761165
applyString(c, l, 0);
11771166
}
11781167

1168+
// coverage tables of a subtable (offsets from it) as numbered tests for the matching of contexts: coverages() reads a
1169+
// list of offsets and returns their numbers, matchCoverage() matches a glyph against a number
1170+
function coverageList(f: Font, subtable: number): { coverages: (o: number, count: number) => number[], matchCoverage: MatchFunc, tests: Array<(glyph: number) => boolean> } {
1171+
const tests: Array<(glyph: number) => boolean> = [];
1172+
const coverages = (o: number, count: number): number[] => Array.from({ length: count }, (_, j) => {
1173+
tests.push(covered(f, subtable + f.u16(o + j * 2)));
1174+
return tests.length - 1;
1175+
});
1176+
return { coverages, matchCoverage: (info, value) => tests[value](info.codepoint), tests };
1177+
}
1178+
11791179
const values16 = (f: Font, o: number, count: number): number[] => Array.from({ length: count }, (_, j) => f.u16(o + j * 2));
11801180

11811181
// the most bits a feature's value takes (HB_OT_MAP_MAX_BITS)
@@ -1501,16 +1501,16 @@ export class MapBuilder {
15011501
}
15021502

15031503
hasFeature(tag: string): boolean {
1504-
return [0, 1].some(t => this.tables[t] && (this.tables[t] as LayoutTable).langSysFeatures(this.langSys[t]).features.has(tag));
1504+
return [0, 1].some(t => this.tables[t]?.langSysFeatures(this.langSys[t]).features.has(tag));
15051505
}
15061506

15071507
compile(): OTMap {
15081508
const m = new OTMap();
15091509
m.chosenScript = [...this.chosenScript] as [string, string];
15101510
m.foundScript = [...this.foundScript] as [boolean, boolean];
15111511

1512-
const langSys = [0, 1].map(t => this.tables[t] ? (this.tables[t] as LayoutTable).langSysFeatures(this.langSys[t]) : { required: -1, features: new Map<string, number>() });
1513-
const requiredTag = [0, 1].map(t => langSys[t].required >= 0 && this.tables[t] ? (this.tables[t] as LayoutTable).featureTag(langSys[t].required) : '');
1512+
const langSys = [0, 1].map(t => this.tables[t]?.langSysFeatures(this.langSys[t]) ?? { required: -1, features: new Map<string, number>() });
1513+
const requiredTag = [0, 1].map(t => langSys[t].required >= 0 ? this.tables[t]?.featureTag(langSys[t].required) ?? '' : '');
15141514
const requiredStage = [0, 0];
15151515

15161516
// sort the features and merge duplicates
@@ -1556,7 +1556,7 @@ export class MapBuilder {
15561556
}
15571557
}
15581558
if (!found && info.flags & FeatureFlags.GLOBAL_SEARCH) for (const t of [0, 1]) {
1559-
const i = this.tables[t] ? (this.tables[t] as LayoutTable).findFeature(info.tag) : -1;
1559+
const i = this.tables[t]?.findFeature(info.tag) ?? -1;
15601560
if (i >= 0) {
15611561
index[t] = i;
15621562
found = true;

0 commit comments

Comments
 (0)