From 855d304b4f81c52ff11e28ba6c374023caf5188d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:05:04 +0100 Subject: [PATCH] fix: stop a crafted winmail.dat from freezing the app --- src/lib/__tests__/tnef.test.ts | 27 +++++++++++++++++++++++++++ src/lib/tnef.ts | 21 ++++++++++++++++++--- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/lib/__tests__/tnef.test.ts b/src/lib/__tests__/tnef.test.ts index f63b42df..e176c35a 100644 --- a/src/lib/__tests__/tnef.test.ts +++ b/src/lib/__tests__/tnef.test.ts @@ -65,6 +65,33 @@ describe('parseTnef', () => { expect(res.htmlBody).toBe('

hi

'); expect(res.attachments[0]).toMatchObject({ name: 'report.pdf', mimeType: 'application/pdf' }); }); + // A sender-chosen value count of 2^32 - 1 over a block with no room for a + // single value used to spin the JS thread on reads that never advance. + it('does not spin on a value count the block cannot hold', () => { + const mapiVar = [ + ...u32(1), + 0x02, 0x01, 0x01, 0x00, // PT_BINARY, id 0x0001 + ...u32(0xffffffff), + 0x00, 0x00, + ]; + const mapiFixed = [ + ...u32(1), + 0x99, 0x10, 0x01, 0x00, // multi-value of an unknown fixed type + ...u32(0xffffffff), + 0x00, 0x00, + ]; + for (const mapi of [mapiVar, mapiFixed]) { + const data = new Uint8Array([ + ...u32(0x223e9f78), 0x00, 0x00, + ...attr(0x01, 0x0002800c, bytes('Hello body')), + ...attr(0x01, 0x00069003, mapi), + ]); + const started = Date.now(); + const res = parseTnef(data); + expect(Date.now() - started).toBeLessThan(1000); + expect(res.body).toBe('Hello body'); + } + }); }); describe('isTnefAttachment', () => { diff --git a/src/lib/tnef.ts b/src/lib/tnef.ts index 84a436b4..42a47203 100644 --- a/src/lib/tnef.ts +++ b/src/lib/tnef.ts @@ -165,6 +165,15 @@ function decodeMAPIString(data: Uint8Array, propType: number): string { return decodeUtf8(data.subarray(0, len)); } +/** + * Read a multi-value count. Every value takes at least four bytes, so a count + * larger than what is left of the block is a lie; clamp it rather than loop + * up to 2^32 times on the JS thread. + */ +function readValueCount(r: BinaryReader): number { + return Math.min(r.readUint32LE(), Math.floor(r.remaining / 4)); +} + function parseMAPIProps(data: Uint8Array): Map { const props = new Map(); const r = new BinaryReader(data); @@ -198,19 +207,25 @@ function parseMAPIProps(data: Uint8Array): Map 0; j++) { - lastValue = readMAPIVarValue(r); + const value = readMAPIVarValue(r); + // A truncated value consumes nothing; retrying it would spin on the + // same bytes for as many rounds as the sender's count asks for. + if (value === null) break; + lastValue = value; } if (!isMultiValue && lastValue) { props.set(propID, { type: propType, value: lastValue }); } } else if (isMultiValue) { if (r.remaining < 4) break; - const valueCount = r.readUint32LE(); + const valueCount = readValueCount(r); for (let j = 0; j < valueCount && r.remaining > 0; j++) { + const before = r.remaining; readMAPIFixedValue(r, baseType); + if (r.remaining >= before) break; } } else { const value = readMAPIFixedValue(r, baseType);