From 8e8e29f511c2fb7c51eca8646893e97be95bd86d Mon Sep 17 00:00:00 2001 From: Felix Boehm <188768+fb55@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:34:24 +0100 Subject: [PATCH] fix(decode): prevent 11-bit consumed overflow for overlong numeric entities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseNumericEntity packs its result as (consumed << 21) | codepoint, giving consumed 11 bits. Numeric entities of 2048+ characters wrapped the shift mod 2^32, corrupting the advance past the entity: decodeHTML could swallow ~2K characters of input or re-emit entity bodies as text, and the strict/XML flavors silently failed to decode instead of emitting U+FFFD. Saturate the packed consumed at 0x7ff and have callers recompute the true length from the input via numericEntityLength() when they see the saturated value. The hot path keeps the packed fast path (one predictable compare); the recompute only runs for pathological 2047+ character entities. Also clamp the streaming EntityDecoder's numeric accumulators to 0x110000 (1 past the Unicode max) so both parsers clamp identically — previously the streaming path relied on float saturation and could pass Infinity to the errors callback — and cross-reference the sync and streaming numeric parsers so fixes are mirrored. Expected decode results for the boundary matrix (2044-4096 digit bodies, decimal/hex, with/without semicolon, all four decode functions) are verified against entities@7.0.1. --- src/decode.spec.ts | 62 ++++++++++++++++++++++++++++++++++ src/decode.ts | 84 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 140 insertions(+), 6 deletions(-) diff --git a/src/decode.spec.ts b/src/decode.spec.ts index a36519a6..57c4333e 100644 --- a/src/decode.spec.ts +++ b/src/decode.spec.ts @@ -180,6 +180,54 @@ describe.each(implementations)("Decode test: %s", (_name, { }); }); + describe("pathologically long numeric entities (11-bit `consumed` overflow)", () => { + /* + * `parseNumericEntity` packs `consumed` into an 11-bit field; long + * digit runs used to wrap the `<< 21` shift and corrupt the advance + * past the entity. Full entity lengths (`#` + [x] + digits + `;`) + * cross the 2047 limit at 2046 decimal / 2045 hex digits, so this + * matrix brackets the boundary on both sides. + * + * Expected strings verified against entities@7.0.1. + */ + const cases = [2044, 2045, 2046, 2047, 2048, 4096].flatMap((digits) => + [false, true].flatMap((isHex) => + [true, false].map((hasSemi) => ({ digits, isHex, hasSemi })), + ), + ); + + it.each( + cases, + )("should decode a body of $digits digits (hex: $isHex, semicolon: $hasSemi)", ({ + digits, + isHex, + hasSemi, + }) => { + const body = (isHex ? "f" : "1").repeat(digits); + const input = `PRE&#${isHex ? "x" : ""}${body}${hasSemi ? ";" : ""}POST`; + const decoded = "PRE�POST"; + + expect(decodeHTML(input)).toBe(decoded); + expect(decodeHTMLAttribute(input)).toBe(decoded); + // Strict modes only decode semicolon-terminated entities. + expect(decodeHTMLStrict(input)).toBe(hasSemi ? decoded : input); + expect(decodeXML(input)).toBe(hasSemi ? decoded : input); + }); + + it("should advance exactly past an overlong entity", () => + expect(decodeHTML(`&#${"1".repeat(2048)};X`)).toBe("�X")); + + it("should not swallow input after an entity with overlong leading zeros", () => { + // The codepoint (38, `&`) is valid; only `consumed` overflows. + const input = `PRE&#${"0".repeat(4090)}38;POST`; + + expect(decodeHTML(input)).toBe("PRE&POST"); + expect(decodeHTMLStrict(input)).toBe("PRE&POST"); + expect(decodeHTMLAttribute(input)).toBe("PRE&POST"); + expect(decodeXML(input)).toBe("PRE&POST"); + }); + }); + it("should parse   followed by < (#852)", () => expect(decodeHTML(" <")).toBe("\u{A0}<")); @@ -427,6 +475,20 @@ describe("EntityDecoder", () => { ).toHaveBeenCalledWith(0x3a); }); + it("should clamp overflowing numeric values before reporting them", () => { + const digits = "9".repeat(2048); + + expect(decoder.write(`&#${digits};`, 1)).toBe(digits.length + 3); + + expect(callback).toHaveBeenCalledTimes(1); + // Consumed: `&` + `#` + digits + `;`. + expect(callback).toHaveBeenCalledWith(0xff_fd, digits.length + 3); + // Clamped to 1 past the Unicode max — not Infinity. + expect( + errorHandlers.validateNumericCharacterReference, + ).toHaveBeenCalledWith(0x11_00_00); + }); + it("should produce an error for numeric entities without digits", () => { expect(decoder.write("&#", 1)).toBe(-1); expect(decoder.end()).toBe(0); diff --git a/src/decode.ts b/src/decode.ts index fba7cff2..8eca8343 100644 --- a/src/decode.ts +++ b/src/decode.ts @@ -205,6 +205,8 @@ export class EntityDecoder { * Parses a hexadecimal numeric entity. * * Equivalent to the `Hexademical character reference state` in the HTML spec. + * + * Mirrors the sync `parseNumericEntity`; fixes here must be mirrored there. * @param input The string containing the entity (or a continuation of the entity). * @param offset The current offset. * @returns The number of characters that were consumed, or -1 if the entity is incomplete. @@ -219,6 +221,8 @@ export class EntityDecoder { ? char - CharCodes.ZERO : (char | TO_LOWER_BIT) - CharCodes.LOWER_A + 10; this.result = this.result * 16 + digit; + // Clamp overflow to 1 past the Unicode max (like `parseNumericEntity`). + if (this.result > 0x10_ff_ff) this.result = 0x11_00_00; this.consumed += 1; offset += 1; } else { @@ -232,6 +236,8 @@ export class EntityDecoder { * Parses a decimal numeric entity. * * Equivalent to the `Decimal character reference state` in the HTML spec. + * + * Mirrors the sync `parseNumericEntity`; fixes here must be mirrored there. * @param input The string containing the entity (or a continuation of the entity). * @param offset The current offset. * @returns The number of characters that were consumed, or -1 if the entity is incomplete. @@ -241,6 +247,8 @@ export class EntityDecoder { const char = input.charCodeAt(offset); if (isNumber(char)) { this.result = this.result * 10 + (char - CharCodes.ZERO); + // Clamp overflow to 1 past the Unicode max (like `parseNumericEntity`). + if (this.result > 0x10_ff_ff) this.result = 0x11_00_00; this.consumed += 1; offset += 1; } else { @@ -630,6 +638,54 @@ function readTrieValue( ); } +/** + * Maximum value of the 11-bit `consumed` field in `parseNumericEntity`'s + * packed result. Callers that see this value must recompute the true + * length via `numericEntityLength`. + */ +const NUMERIC_CONSUMED_MAX = 0x7_ff; + +/** + * Recompute the length of a numeric entity directly from the input. + * + * Only used when the `consumed` field of `parseNumericEntity`'s packed + * result saturates at `NUMERIC_CONSUMED_MAX` (entities of 2047+ + * characters), so this cold path favors simplicity over speed. + * @param input The input string. + * @param numberStart Index of the `#` character. + * @returns The number of characters in the entity, starting at (and + * including) the `#`, including the terminating semicolon if + * present. + */ +function numericEntityLength(input: string, numberStart: number): number { + const inputLength = input.length; + let offset = numberStart + 1; // Skip "#" + let isHex = false; + + if (offset < inputLength) { + const first = input.charCodeAt(offset); + if (first === CharCodes.LOWER_X || first === CharCodes.UPPER_X) { + isHex = true; + offset += 1; + } + } + + while ( + offset < inputLength && + (isNumber(input.charCodeAt(offset)) || + (isHex && isHexadecimalCharacter(input.charCodeAt(offset)))) + ) { + offset += 1; + } + + // Include the semicolon when present, mirroring `parseNumericEntity`. + if (offset < inputLength && input.charCodeAt(offset) === CharCodes.SEMI) { + offset += 1; + } + + return offset - numberStart; +} + /** * Parse a numeric entity (`&#DDD;` or `&#xHHH;`). * @@ -637,6 +693,9 @@ function readTrieValue( * field fits any valid Unicode value (max 0x10FFFF), and packing both into * one integer avoids the file-scope `_numericCp` tuple-by-globals pattern. * Returns 0 when no digits were found. + * + * Mirrors the streaming `stateNumericDecimal` / `stateNumericHex` in + * `EntityDecoder`; fixes here must be mirrored there. * @param input The input string. * @param numberStart Index of the `#` character. * @param inputLength Cached `input.length`. @@ -691,14 +750,19 @@ function parseNumericEntity( * like `�` would silently truncate to a valid-looking * codepoint instead of mapping to U+FFFD via `replaceCodePoint`. * - * `consumed` fits 11 bits, which covers any practical entity body. - * Pathologically long digit runs (>2047 chars) overflow the field, - * but those inputs already produce U+FFFD for the entity value, so - * the only observable difference is a slightly wrong advance — never - * an incorrect emitted character. + * `consumed` gets 11 bits, which covers any practical entity body. + * Pathological digit runs would overflow the field and wrap the `<< 21`, + * corrupting the advance past the entity — swallowing or re-emitting + * input, and not only around U+FFFD outputs: with leading zeros (e.g. + * `�…038;`) `cp` stays perfectly valid. Instead, `consumed` is + * saturated at NUMERIC_CONSUMED_MAX; callers seeing that value + * recompute the true length with `numericEntityLength` — a cold path + * that keeps the common case branch-predictable and allocation-free. */ if (cp > 0x10_ff_ff) cp = 0x11_00_00; - return ((offset - numberStart) << 21) | cp; + let consumed = offset - numberStart; + if (consumed > NUMERIC_CONSUMED_MAX) consumed = NUMERIC_CONSUMED_MAX; + return (consumed << 21) | cp; } /** @@ -738,6 +802,10 @@ function decodeWithTrie( if (firstChar === CharCodes.NUM) { const packed = parseNumericEntity(input, entityStart, inputLength); consumed = packed >>> 21; + // Saturated `consumed` — recompute the true length (cold path). + if (consumed === NUMERIC_CONSUMED_MAX) { + consumed = numericEntityLength(input, entityStart); + } // In strict mode, require semicolon termination. if ( isStrict && @@ -1014,6 +1082,10 @@ export function decodeXML(xmlString: string): string { xmlString.length, ); consumed = packed >>> 21; + // Saturated `consumed` — recompute the true length (cold path). + if (consumed === NUMERIC_CONSUMED_MAX) { + consumed = numericEntityLength(xmlString, start); + } // XML is always strict — require semicolon. if ( consumed === 0 ||