Finding 5 — the MIME parser runs on attacker-controlled input: the inner content recovered after decrypt/verify is whatever the sender put there. Upstream had no depth limit on nested multiparts and no size cap anywhere. Verified against the unpatched upstream parser with the same input: UPSTREAM CRASHED: RangeError - Maximum call stack size exceeded UPSTREAM: 65MB accepted (no size cap) So this was a live decrypt-time DoS reachable by anyone who can send mail. Caps added: depth 20, parts 500, bytes 64 MB — generous enough that no legitimate message comes close (real mail nests 3-4 levels). Past a limit a subtree degrades to a leaf rather than throwing, so one pathological branch doesn't discard the legitimate parts above it. Oversize input is refused outright rather than truncated: half a MIME tree parses into misleading nonsense, and showing part of a message is worse than saying no. Both bodyStructure walkers in smime-detect.js are capped too — those run on server-supplied structure BEFORE any decrypt/verify gate. Finding 4 — hardened, not eliminated, per the agreed scope. Unlocked CryptoKeys still live in durable IndexedDB rather than memory; moving them would mean refactoring how the plugin shares state across iframes and risking the unlock->decrypt path just verified. What changed instead: - Removed the lockOnLogout opt-out from the logout/account-switch wipes. A non-extractable key cannot be exported but can still be USED, so a handle outliving the session lets anyone with the browser profile decrypt mail without knowing the passphrase. That is not a preference to toggle off. - Added a best-effort wipe on pagehide and beforeunload to narrow the window in which a usable handle exists on disk. Best-effort by nature: an IndexedDB write may not complete during teardown and neither event fires on a crash — which is precisely why the boot wipe in activate() remains the load-bearing control. - Deliberately NOT wiping on visibilitychange: tabbing away would drop the unlock and force a passphrase re-entry every time, which trains users into turning S/MIME off entirely. - Dropped the now-dead lockOnLogout setting from the manifest. A toggle that silently does nothing is worse than no toggle. Tests: 49 unit assertions + 28 round trip. The round trip now feeds genuinely hostile MIME through the real parser (5000-level nesting, 5000 siblings, 65 MB) and still confirms a normal multipart/alternative parses correctly. Full crypto round trip unchanged and passing, so neither fix broke S/MIME. Findings 6, 7, 8 and 9 remain open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
221 lines
7.6 KiB
JavaScript
221 lines
7.6 KiB
JavaScript
/**
|
|
* Minimal RFC 5322 / MIME parser used only for the inner content recovered
|
|
* after decryption / signature-stripping. We need just enough to pull out the
|
|
* best-alternative text/html body and any leaf attachments; the host
|
|
* re-sanitizes returned HTML, so this never has to be a hardened renderer.
|
|
*/
|
|
|
|
const decoder = new TextDecoder('utf-8', { fatal: false });
|
|
|
|
// ─── VNC: resource limits (audit finding 5) ────────────────────────────
|
|
//
|
|
// This parser runs on attacker-controlled input: the inner MIME recovered after
|
|
// decrypt/verify is whatever the sender put there. Upstream had no depth limit
|
|
// on nested multiparts and no size cap anywhere, so a crafted message could
|
|
// blow the stack or exhaust memory — a decrypt-time DoS reachable by anyone who
|
|
// can send you mail.
|
|
//
|
|
// Limits are generous enough that no legitimate message hits them: real mail
|
|
// nests maybe 3-4 levels (mixed > alternative > related), and 64 MB is far above
|
|
// any sane attachment set surviving base64 in a single message.
|
|
const MAX_DEPTH = 20;
|
|
const MAX_PARTS = 500;
|
|
const MAX_BYTES = 64 * 1024 * 1024;
|
|
|
|
/** Parse raw inner MIME bytes into { html, text, attachments }. */
|
|
export function parseMime(bytes) {
|
|
if (bytes.length > MAX_BYTES) {
|
|
// Refuse rather than truncate: half a MIME tree parses into misleading
|
|
// nonsense, and silently showing part of a message is worse than saying no.
|
|
throw new Error(
|
|
`Refusing to parse: message exceeds ${Math.round(MAX_BYTES / 1024 / 1024)} MB`,
|
|
);
|
|
}
|
|
const text = binaryString(bytes);
|
|
const node = parseEntity(text, 0, { parts: 0 });
|
|
const out = { html: '', text: '', attachments: [] };
|
|
collect(node, out);
|
|
// Fallback for non-MIME inner content (e.g. messages signed/encrypted by
|
|
// OpenSSL or older clients where the protected payload is raw text with no
|
|
// Content-Type). If structured parsing produced no renderable body, surface
|
|
// the decoded bytes as plain text so the message is never shown blank.
|
|
if (!out.html && !out.text) {
|
|
const raw = decoder.decode(bytes).trim();
|
|
if (raw) out.text = raw;
|
|
}
|
|
return out;
|
|
}
|
|
|
|
// Treat bytes as latin1 so byte boundaries survive; decode per-part by charset.
|
|
function binaryString(bytes) {
|
|
let s = '';
|
|
for (let i = 0; i < bytes.length; i++) s += String.fromCharCode(bytes[i]);
|
|
return s;
|
|
}
|
|
|
|
// VNC: `depth` and the shared `budget` bound the recursion (finding 5). Past
|
|
// either limit the node is returned as a leaf rather than throwing, so a
|
|
// pathological subtree degrades to "not rendered" instead of failing the whole
|
|
// message — the parts above it are still legitimate and worth showing.
|
|
function parseEntity(raw, depth = 0, budget = { parts: 0 }) {
|
|
const sepMatch = raw.match(/\r?\n\r?\n/);
|
|
const headerText = sepMatch ? raw.slice(0, sepMatch.index) : raw;
|
|
const body = sepMatch ? raw.slice(sepMatch.index + sepMatch[0].length) : '';
|
|
|
|
const headers = parseHeaders(headerText);
|
|
const ctRaw = headers['content-type'] || 'text/plain';
|
|
const { type, params } = parseContentType(ctRaw);
|
|
const cte = (headers['content-transfer-encoding'] || '7bit').trim().toLowerCase();
|
|
const disposition = (headers['content-disposition'] || '').toLowerCase();
|
|
|
|
const node = { type, params, cte, disposition, headers, body, children: [] };
|
|
|
|
if (type.startsWith('multipart/') && params.boundary && depth < MAX_DEPTH) {
|
|
for (const seg of splitMultipart(body, params.boundary)) {
|
|
if (budget.parts >= MAX_PARTS) break;
|
|
budget.parts += 1;
|
|
node.children.push(parseEntity(seg, depth + 1, budget));
|
|
}
|
|
}
|
|
return node;
|
|
}
|
|
|
|
function parseHeaders(headerText) {
|
|
const unfolded = headerText.replace(/\r?\n[ \t]+/g, ' ');
|
|
const headers = {};
|
|
for (const line of unfolded.split(/\r?\n/)) {
|
|
const idx = line.indexOf(':');
|
|
if (idx <= 0) continue;
|
|
const name = line.slice(0, idx).trim().toLowerCase();
|
|
const value = line.slice(idx + 1).trim();
|
|
headers[name] = headers[name] ? `${headers[name]}, ${value}` : value;
|
|
}
|
|
return headers;
|
|
}
|
|
|
|
function parseContentType(value) {
|
|
const parts = value.split(';');
|
|
const type = parts[0].trim().toLowerCase();
|
|
const params = {};
|
|
for (let i = 1; i < parts.length; i++) {
|
|
const eq = parts[i].indexOf('=');
|
|
if (eq < 0) continue;
|
|
const k = parts[i].slice(0, eq).trim().toLowerCase();
|
|
let v = parts[i].slice(eq + 1).trim();
|
|
if (v.startsWith('"') && v.endsWith('"')) v = v.slice(1, -1);
|
|
params[k] = v;
|
|
}
|
|
return { type, params };
|
|
}
|
|
|
|
function splitMultipart(body, boundary) {
|
|
const delim = `--${boundary}`;
|
|
const parts = [];
|
|
const segments = body.split(delim);
|
|
for (let i = 1; i < segments.length; i++) {
|
|
let seg = segments[i];
|
|
if (seg.startsWith('--')) break; // closing delimiter
|
|
seg = seg.replace(/^\r?\n/, '').replace(/\r?\n$/, '');
|
|
parts.push(seg);
|
|
}
|
|
return parts;
|
|
}
|
|
|
|
function decodeBody(node) {
|
|
const { cte, body } = node;
|
|
if (cte === 'base64') {
|
|
const cleaned = body.replace(/[^A-Za-z0-9+/=]/g, '');
|
|
try {
|
|
const bin = atob(cleaned);
|
|
const bytes = new Uint8Array(bin.length);
|
|
for (let i = 0; i < bin.length; i++) bytes[i] = bin.charCodeAt(i);
|
|
return bytes;
|
|
} catch {
|
|
return new Uint8Array(0);
|
|
}
|
|
}
|
|
if (cte === 'quoted-printable') {
|
|
return qpDecode(body);
|
|
}
|
|
// 7bit / 8bit / binary — body is a latin1 binary string
|
|
const bytes = new Uint8Array(body.length);
|
|
for (let i = 0; i < body.length; i++) bytes[i] = body.charCodeAt(i) & 0xff;
|
|
return bytes;
|
|
}
|
|
|
|
function qpDecode(input) {
|
|
const out = [];
|
|
const cleaned = input.replace(/=\r?\n/g, ''); // soft line breaks
|
|
for (let i = 0; i < cleaned.length; i++) {
|
|
const c = cleaned[i];
|
|
if (c === '=' && i + 2 < cleaned.length) {
|
|
const hex = cleaned.substr(i + 1, 2);
|
|
if (/^[0-9A-Fa-f]{2}$/.test(hex)) {
|
|
out.push(parseInt(hex, 16));
|
|
i += 2;
|
|
continue;
|
|
}
|
|
}
|
|
out.push(cleaned.charCodeAt(i) & 0xff);
|
|
}
|
|
return new Uint8Array(out);
|
|
}
|
|
|
|
function decodeText(node) {
|
|
const bytes = decodeBody(node);
|
|
const charset = (node.params.charset || 'utf-8').toLowerCase();
|
|
try {
|
|
return new TextDecoder(charset, { fatal: false }).decode(bytes);
|
|
} catch {
|
|
return decoder.decode(bytes);
|
|
}
|
|
}
|
|
|
|
function filenameFor(node) {
|
|
const cd = node.headers['content-disposition'] || '';
|
|
const m = cd.match(/filename\*?=(?:"([^"]+)"|([^;]+))/i);
|
|
if (m) return (m[1] || m[2] || '').trim();
|
|
if (node.params.name) return node.params.name;
|
|
return 'attachment';
|
|
}
|
|
|
|
function collect(node, out) {
|
|
const { type, disposition } = node;
|
|
const isAttachment = disposition.includes('attachment') ||
|
|
(!type.startsWith('text/') && !type.startsWith('multipart/'));
|
|
|
|
if (type.startsWith('multipart/')) {
|
|
if (type === 'multipart/alternative') {
|
|
// Prefer the richest alternative; collect text+html, last wins per type.
|
|
for (const child of node.children) collect(child, out);
|
|
} else {
|
|
for (const child of node.children) collect(child, out);
|
|
}
|
|
return;
|
|
}
|
|
|
|
if (type === 'text/html' && !isAttachment) {
|
|
out.html = decodeText(node);
|
|
return;
|
|
}
|
|
if (type === 'text/plain' && !isAttachment) {
|
|
out.text = decodeText(node);
|
|
return;
|
|
}
|
|
|
|
// Leaf attachment
|
|
const bytes = decodeBody(node);
|
|
out.attachments.push({
|
|
name: filenameFor(node),
|
|
type: type || 'application/octet-stream',
|
|
size: bytes.length,
|
|
dataUrl: bytesToDataUrl(bytes, type || 'application/octet-stream'),
|
|
});
|
|
}
|
|
|
|
function bytesToDataUrl(bytes, type) {
|
|
let binary = '';
|
|
for (let i = 0; i < bytes.length; i++) binary += String.fromCharCode(bytes[i]);
|
|
return `data:${type};base64,${btoa(binary)}`;
|
|
}
|