mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-02 16:15:39 +00:00
fix: stop a crafted winmail.dat from freezing the tab
A MAPI property in a TNEF attachment declares how many values follow. The parser looped that many times even when a value could not be read and nothing was consumed, so a 31-byte winmail.dat claiming 0xFFFFFFFF values froze the tab for about 13 seconds on every open, longer with more such properties. The count is now clamped to what the block can hold and the loop stops as soon as a value consumes no bytes.
This commit is contained in:
Binary file not shown.
+18
-3
@@ -165,6 +165,15 @@ function decodeMAPIString(data: Uint8Array, propType: number): string {
|
||||
return new TextDecoder('utf-8').decode(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 UI thread.
|
||||
*/
|
||||
function readValueCount(r: BinaryReader): number {
|
||||
return Math.min(r.readUint32LE(), Math.floor(r.remaining / 4));
|
||||
}
|
||||
|
||||
/** Parse MAPI properties from a raw attribute data block */
|
||||
function parseMAPIProps(data: Uint8Array): Map<number, { type: number; value: Uint8Array | number | null }> {
|
||||
const props = new Map<number, { type: number; value: Uint8Array | number | null }>();
|
||||
@@ -200,19 +209,25 @@ function parseMAPIProps(data: Uint8Array): Map<number, { type: number; value: Ui
|
||||
if (isVarLengthType(baseType)) {
|
||||
// Variable-length types always have a value count (1 for single-value)
|
||||
if (r.remaining < 4) break;
|
||||
const valueCount = r.readUint32LE();
|
||||
const valueCount = readValueCount(r);
|
||||
let lastValue: Uint8Array | null = null;
|
||||
for (let j = 0; j < valueCount && r.remaining > 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);
|
||||
|
||||
Reference in New Issue
Block a user