From 77f90dfa646957c866d1820167fe261c40cb62fb Mon Sep 17 00:00:00 2001 From: Local Dev Date: Sun, 4 Oct 2026 04:09:45 +0200 Subject: [PATCH] Aegis: smaller hardening from the 0.32 audit - Electrum: a reply larger than 8 MB ends the connection; a server streaming without newlines used to grow the buffer without bound and re-parse it on every chunk, on the Electron main thread. - Overlay amounts are formatted from integer strings: 0.1 ETH read 0.100000000000000006, and 1 ETH + 1 wei read 1. - The panel escapes a broadcast txid shown without an explorer link. - A TPM refusal hands back only its own PIN attempt, not the count before it, which could undo guesses made in parallel. - BCH WIF imports must carry this network's version byte and a valid compression flag; a 64-byte Solana keypair must have a public half that matches its secret half. - WizardConnect: the overlay shows the network fee (exact, as SIGHASH_UTXOS commits to the inputs), marks outputs to this wallet's own addresses, lists every token input and every output instead of "... and N more" (more than 30 is refused), and a very high fee or token inputs get the danger button. - DAI permits: expiry 0 reads "never expires", and the allowed flag is read the way the encoder signs it. --- bundled-addons/aegis/index.js | 63 +++++++++++++++++------ bundled-addons/aegis/lib/electrum.js | 14 ++++- bundled-addons/aegis/lib/import-derive.js | 9 ++++ bundled-addons/aegis/panel.js | 4 +- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/bundled-addons/aegis/index.js b/bundled-addons/aegis/index.js index ee54ed37..af38bfcf 100644 --- a/bundled-addons/aegis/index.js +++ b/bundled-addons/aegis/index.js @@ -955,8 +955,12 @@ function deriveCashaddrFromWif(wif, prefix) { throw new Error("invalid WIF format (base58check decode failed)"); } if (raw.length !== 33 && raw.length !== 34) throw new Error(`bad WIF length ${raw.length}`); - // First byte is version (network); we allow any — BCH mainnet uses 0x80, - // testnet 0xEF. Both round-trip through the same address derivation below. + // Version byte must be this network's (0x80 mainnet, 0xEF testnet) and a + // 34-byte WIF must end in the 0x01 compression flag; anything else used to + // be accepted and could derive an address the key's owner never used. + const want = prefix === "bitcoincash" ? 0x80 : 0xef; + if (raw[0] !== want) throw new Error(`this WIF is for another network (version 0x${raw[0].toString(16)})`); + if (raw.length === 34 && raw[33] !== 0x01) throw new Error("bad WIF compression flag"); const priv = raw.slice(1, 33); const compressed = raw.length === 34; // trailing 0x01 marker const pub = d.secp256k1.getPublicKey(priv, compressed); @@ -1000,6 +1004,16 @@ function buildWcApproval(payload) { || !Array.isArray(tx.sourceOutputs) || tx.sourceOutputs.length !== inner.inputs.length) { return { unreadable: true, dappName }; } + // Every output is listed; a request with more than fit on an overlay is + // refused rather than summarised as "… and N more". + if (inner.outputs.length > 30 || tx.sourceOutputs.length > 30) return { unreadable: true, dappName }; + // Scripts this wallet has handed out, to tell change from a payment. + const ownScripts = new Set(); + try { + const w = ctx.runtimes.get(payload?.walletId)?.adapter?._wallet; + for (const e of (w?.state?.watched?.values?.() || [])) if (e && e.script) ownScripts.add(Buffer.from(e.script).toString("hex")); + } catch {} + let inSum = null; const inputCount = Array.isArray(req.inputPaths) ? req.inputPaths.length : (Array.isArray(inner?.inputs) ? inner.inputs.length : "?"); const rows = [{ label: "Wallet", value: walletName }, { label: "Inputs", value: String(inputCount) }]; if (says) rows.unshift({ label: "The dapp says", value: `“${says}”` }); @@ -1023,17 +1037,18 @@ function buildWcApproval(payload) { const tokenLines = []; srcs.forEach((o, i) => { try { inTotal += BigInt(o.valueSatoshis ?? 0); } catch {} + inSum = inTotal; if (o.token) tokenLines.push(`input #${i + 1}${tokenText(o.token)}`); }); rows.push({ label: "Spending", value: `${fmtBch(Number(inTotal))} BCH from ${srcs.length} input${srcs.length === 1 ? "" : "s"}` }); - if (tokenLines.length) rows.push({ label: "Tokens spent", value: tokenLines.slice(0, 8).join("\n") + (tokenLines.length > 8 ? `\n… and ${tokenLines.length - 8} more` : ""), mono: true, strong: true }); + if (tokenLines.length) rows.push({ label: "Tokens spent", value: tokenLines.join("\n"), mono: true, strong: true }); } } catch {} try { const outs = inner?.outputs || []; let total = 0n; for (const o of outs) { try { total += BigInt(o.valueSatoshis ?? 0); } catch {} } - outs.slice(0, 8).forEach((o, i) => { + outs.forEach((o, i) => { const lb = o.lockingBytecode; const hexScript = typeof lb === "string" ? lb.toLowerCase() : Buffer.from(lb || []).toString("hex"); let addr = null; @@ -1041,14 +1056,21 @@ function buildWcApproval(payload) { else if (/^a914[0-9a-f]{40}87$/.test(hexScript)) addr = ctx.d.cashaddr.encode(prefix, 1, ctx.d.tx.fromHex(hexScript.slice(4, 44))); const v = BigInt(o.valueSatoshis ?? 0); const token = tokenText(o.token); - rows.push({ label: `Output #${i + 1}`, value: `${fmtBch(Number(v))} BCH${token} → ${addr || (hexScript.startsWith("6a") ? "OP_RETURN data" : "script " + hexScript.slice(0, 24) + "…")}`, mono: true }); + const mine = ownScripts.has(hexScript) ? " (your address)" : ""; + rows.push({ label: `Output #${i + 1}`, value: `${fmtBch(Number(v))} BCH${token} → ${addr || (hexScript.startsWith("6a") ? "OP_RETURN data" : "script " + hexScript.slice(0, 24) + "…")}${mine}`, mono: true }); }); - if (outs.length > 8) rows.push({ label: "Outputs", value: `… and ${outs.length - 8} more` }); if (outs.length) rows.push({ label: "Total out", value: `${fmtBch(Number(total))} BCH`, strong: true }); + // The fee is what the inputs carry beyond the outputs. The values are + // trustworthy (SIGHASH_UTXOS commits to them), so this is exact. + if (inSum != null) { + const fee = inSum - total; + rows.push({ label: "Network fee", value: fee >= 0n ? `${fmtBch(Number(fee))} BCH` : "negative — the transaction cannot be valid", strong: fee > 100000n }); + if (fee > 100000n) rows.unshift({ label: "Warning", value: `This transaction pays ${fmtBch(Number(fee))} BCH in fees — far more than a normal transaction.`, strong: true }); + } } catch {} rows.push({ label: "After signing", value: tx.broadcast ? "the dapp broadcasts it" : "signed hex is returned to the dapp" }); rows.push({ label: "Sighash", value: "ALL | FORKID | UTXOS" }); - return { body: `A site paired over WizardConnect asks you to sign a Bitcoin Cash transaction. Paired from: ${dappName}.`, rows, dappName, origin: paired && paired !== "panel" ? paired : "WizardConnect" }; + return { body: `A site paired over WizardConnect asks you to sign a Bitcoin Cash transaction. Paired from: ${dappName}.`, rows, dappName, risky: rows.some((r) => r.label === "Warning" || r.label === "Tokens spent"), origin: paired && paired !== "panel" ? paired : "WizardConnect" }; } // ---- per-purpose default wallets ------------------------------------------- @@ -1437,9 +1459,14 @@ function shortAddr(addr) { const fmtBch = (sats) => (Number(sats) / 1e8).toFixed(8).replace(/(\.\d*?[1-9])0+$|\.0+$/, "$1"); const fmtTrx = (sun) => (Number(sun) / 1e6).toFixed(6).replace(/(\.\d*?[1-9])0+$|\.0+$/, "$1"); +// Exact: integer base units are formatted as strings. Float division showed +// 0.1 ETH as 0.100000000000000006, rounded 1 ETH + 1 wei to "1", and put +// Sia's 24-decimal amounts in exponent notation. function fmtValue(units, decimals) { + const t = typeof units === "bigint" ? units.toString() : String(units ?? "0").trim(); + if (/^-?\d+$/.test(t)) return fmtTokenAmount(t, decimals); const n = Number(units) / Math.pow(10, decimals); - return n.toFixed(decimals).replace(/(\.\d*?[1-9])0+$|\.0+$/, "$1"); + return n.toFixed(Math.min(20, decimals)).replace(/(\.\d*?[1-9])0+$|\.0+$/, "$1"); } // BigInt-safe display for SPL token amounts (raw units in u64 strings). function fmtTokenAmount(rawStr, decimals) { @@ -2745,9 +2772,11 @@ function registerPanelMessages(api) { const r = await tpmPin.open(blob.hw.key, blob.hw.wrapped, pin); if (r.ok) secret = r.secret; else if (r.code === "locked" || r.code === "error") { - // Not a verdict on the PIN: give the attempt back. - api.storage.set("aegis/pin/failCount", st.fails); - api.storage.set("aegis/pin/failLast", st.last); + // Not a verdict on the PIN: give this one attempt back (only this + // one — restoring the earlier count would also undo guesses made + // in parallel meanwhile). + api.storage.set("aegis/pin/failCount", Math.max(0, pinFails(api).fails - 1)); + if (st.fails === 0) api.storage.set("aegis/pin/failLast", st.last); return { ok: false, remaining: Math.max(0, PIN_MAX_FAILS - st.fails), lockedMs: 0, hwLocked: r.code === "locked", error: r.code === "locked" ? "The security chip is refusing PINs for a few minutes after too many wrong ones. Use the master password, or wait." : "The security chip did not answer. Use the master password." }; } else if (r.code === "missing") { @@ -3563,9 +3592,11 @@ function describePermit(td) { // The variant is chosen by the declared type, never by what the message // happens to carry. if (has("holder", "spender", "nonce", "expiry", "allowed")) { - // DAI-style: a bool, encoded by truthiness — so is this. - items.push({ token: td.domain?.verifyingContract, amount: msg.allowed ? UNLIMITED : 0n }); - rows.push({ label: "Valid until", value: when(msg.expiry) }); + // DAI-style: read the bool exactly as lib/eip712.js encodes it. + const allowed = msg.allowed === true || msg.allowed === 1 || msg.allowed === "true" || msg.allowed === "1"; + items.push({ token: td.domain?.verifyingContract, amount: allowed ? UNLIMITED : 0n }); + // DAI treats expiry 0 as "no expiry", not 1970. + rows.push({ label: "Valid until", value: big(msg.expiry) === 0n ? "never expires (expiry 0)" : when(msg.expiry) }); } else if (has("owner", "spender", "value", "deadline")) { // EIP-2612. items.push({ token: td.domain?.verifyingContract, amount: msg.value }); @@ -4615,13 +4646,13 @@ module.exports = { // keys fell back to a lone "OK" button whose id never matched, so // every WizardConnect signing request was refused, and the HTML // body was shown as literal markup. - const { body, rows, dappName, unreadable, origin: wcOrigin } = buildWcApproval(payload); + const { body, rows, dappName, unreadable, origin: wcOrigin, risky: wcRisky } = buildWcApproval(payload); if (unreadable) { api.log(`wc sign: refused an unreadable request from ${dappName}`); return { approved: false }; } const pick = await api.approvalModal({ title: "Sign a Bitcoin Cash transaction (WizardConnect)?", origin: wcOrigin, body, rows, - actions: [{ id: "approve", label: "Sign", primary: true }], + actions: [{ id: "approve", label: wcRisky ? "Sign anyway" : "Sign", primary: !wcRisky, danger: !!wcRisky }], }); if (pick !== "approve") return { approved: false }; try { await requireDappTxPin(dappName, "Bitcoin Cash transaction (WizardConnect)"); } diff --git a/bundled-addons/aegis/lib/electrum.js b/bundled-addons/aegis/lib/electrum.js index e5e3c6d5..ddd96568 100644 --- a/bundled-addons/aegis/lib/electrum.js +++ b/bundled-addons/aegis/lib/electrum.js @@ -57,14 +57,26 @@ module.exports = function makeElectrum({ WebSocket, log = () => {} }) { } _onData(chunk) { this.buf += chunk; + // A server that streams without newlines used to grow this buffer + // without bound on the Electron main thread. No legitimate reply is + // anywhere near this size. + if (this.buf.length > 8 * 1024 * 1024) { + this.buf = ""; + try { this.ws && this.ws.close(); } catch {} + for (const p of this.pending.values()) p.reject(new Error("electrum server sent an oversized reply")); + this.pending.clear(); + return; + } let nl; while ((nl = this.buf.indexOf("\n")) >= 0) { const line = this.buf.slice(0, nl).trim(); this.buf = this.buf.slice(nl + 1); if (line) this._handleLine(line); } + // Re-parsing the whole tail on every chunk was quadratic; only try + // once it could be a complete JSON object. const rest = this.buf.trim(); - if (rest) { try { JSON.parse(rest); this._handleLine(rest); this.buf = ""; } catch {} } + if (rest && rest.endsWith("}")) { try { JSON.parse(rest); this._handleLine(rest); this.buf = ""; } catch {} } } _handleLine(line) { let msg; diff --git a/bundled-addons/aegis/lib/import-derive.js b/bundled-addons/aegis/lib/import-derive.js index 2462cd47..2d425784 100644 --- a/bundled-addons/aegis/lib/import-derive.js +++ b/bundled-addons/aegis/lib/import-derive.js @@ -221,11 +221,19 @@ module.exports = function makeImportDerive({ const pub = ed25519.getPublicKey(sk); return base58check.encodeBase58(pub); } + // A 64-byte Solana keypair is seed || public key. The second half was + // ignored, so a corrupted or mismatched export imported silently as some + // other address; it must match the key the seed produces. + function checkKeypairHalf(full, pub) { + if (full.length !== 64) return; + for (let i = 0; i < 32; i++) if (full[32 + i] !== pub[i]) throw new Error("this 64-byte Solana key is inconsistent: its public half does not match its secret half"); + } function deriveSolFromPrivHex(hex) { const priv = fromHex(hex); if (priv.length !== 32 && priv.length !== 64) throw new Error("SOL private key must be 32 or 64 bytes hex"); const seed = priv.length === 64 ? priv.slice(0, 32) : priv; const pub = ed25519.getPublicKey(seed); + checkKeypairHalf(priv, pub); return base58check.encodeBase58(pub); } function deriveSolFromBase58(b58) { @@ -233,6 +241,7 @@ module.exports = function makeImportDerive({ if (bytes.length !== 32 && bytes.length !== 64) throw new Error("SOL private key base58 must decode to 32 or 64 bytes"); const seed = bytes.length === 64 ? bytes.slice(0, 32) : bytes; const pub = ed25519.getPublicKey(seed); + checkKeypairHalf(bytes, pub); return base58check.encodeBase58(pub); } diff --git a/bundled-addons/aegis/panel.js b/bundled-addons/aegis/panel.js index 683e2f93..63ddb3df 100644 --- a/bundled-addons/aegis/panel.js +++ b/bundled-addons/aegis/panel.js @@ -2212,7 +2212,7 @@ function openConsolidateModal() { const bad = res.results.filter((r) => !r.ok); const explorer = sel()?.explorerTx || ""; const okRows = ok.map((r) => { - const link = explorer && r.txid ? `${esc(r.txid.slice(0, 16))}…` : (r.txid || ""); + const link = explorer && r.txid ? `${esc(r.txid.slice(0, 16))}…` : esc(r.txid || ""); return `
${esc(r.label)}
Sent — ${link}
✓
@@ -2324,7 +2324,7 @@ async function renderConsolidateInline(hostEl) { const explorer = sel()?.explorerTx || ""; const rowsResult = res.results.map((r) => { if (r.ok) { - const link = explorer && r.txid ? `${esc(r.txid.slice(0, 16))}…` : (r.txid || ""); + const link = explorer && r.txid ? `${esc(r.txid.slice(0, 16))}…` : esc(r.txid || ""); return `
${esc(r.label)}
Sent — ${link}
✓