diff --git a/bundled-addons/aegis/index.js b/bundled-addons/aegis/index.js index 8fc06d98..e55fa4fe 100644 --- a/bundled-addons/aegis/index.js +++ b/bundled-addons/aegis/index.js @@ -3164,13 +3164,50 @@ const ETH_RPC_NEVER = /^(eth_sign|eth_signTransaction|eth_sendTransaction|eth_ac // PermitBatch / PermitTransferFrom / PermitBatchTransferFrom). Returns the // overlay rows that say who may spend what until when, or null when the // typed data is not a spending approval. +// What the signature actually covers. The EIP-712 encoder reads only the +// fields each type declares; anything else in `message` is ignored by the +// digest but was read by the overlay, so a dapp could add a decoy +// `allowed: false` or `details: {amount: "1"}` beside an unlimited permit and +// have the overlay show the decoy. Everything shown is built from this view. +function signedView(td) { + const types = (td && typeof td.types === "object" && td.types) || {}; + let dropped = 0; + const walk = (type, v) => { + const arr = /\[\d*\]$/.exec(type); + if (arr) { + const inner = type.slice(0, arr.index); + return Array.isArray(v) ? v.map((x) => walk(inner, x)) : v; + } + const fields = types[type]; + if (!Array.isArray(fields)) return v; // atomic + const src = (v && typeof v === "object") ? v : {}; + for (const k of Object.keys(src)) if (!fields.some((f) => f && f.name === k)) dropped++; + const out = {}; + for (const f of fields) if (f && typeof f.name === "string") out[f.name] = walk(String(f.type), src[f.name]); + return out; + }; + return { message: walk(String(td?.primaryType || ""), td?.message), dropped }; +} + function describePermit(td) { const primary = String(td?.primaryType || ""); if (!/^Permit/.test(primary)) return null; - const msg = (td && typeof td.message === "object" && td.message) || {}; + const msg = signedView(td).message || {}; + const declared = (t) => (Array.isArray(td?.types?.[t]) ? td.types[t].map((f) => f && f.name) : []); + const pf = declared(primary); + const has = (...names) => names.every((n) => pf.includes(n)); const UNLIMITED = 1n << 159n; // ≥ half of uint160 covers Permit2's max and every uint256 max + // No token has a supply anywhere near this, so an amount this large is an + // unlimited approval in all but name (2^96 ≈ 7.9e28 base units: 79 billion + // tokens at 18 decimals). + const HUGE = 1n << 96n; const big = (v) => { try { return BigInt(v); } catch { return null; } }; - const amountText = (v) => { const b = big(v); return b === null ? String(v) : (b >= UNLIMITED ? "UNLIMITED (∞)" : b.toString() + " units"); }; + const amountText = (v) => { + const b = big(v); + if (b === null) return String(v); + if (b >= UNLIMITED) return "UNLIMITED (∞)"; + return b.toString() + " units" + (b >= HUGE ? " (effectively unlimited)" : ""); + }; const when = (v) => { const b = big(v); if (b === null) return String(v); @@ -3179,23 +3216,34 @@ function describePermit(td) { }; const rows = []; let risky = false; - const spender = msg.spender; + const spender = has("spender") ? msg.spender : null; rows.push({ label: "Spender", value: spender ? String(spender) : "(none named — anyone holding this signature)", mono: true, strong: true }); if (!spender) risky = true; const items = []; - if (primary === "Permit") { - // EIP-2612 has `value`; DAI has `allowed: true` (always unlimited). - if ("allowed" in msg) items.push({ token: td.domain?.verifyingContract, amount: msg.allowed ? UNLIMITED : 0n }); - else items.push({ token: td.domain?.verifyingContract, amount: msg.value }); - if (msg.deadline != null || msg.expiry != null) rows.push({ label: "Valid until", value: when(msg.deadline ?? msg.expiry) }); - } else { - const list = Array.isArray(msg.details) ? msg.details : (msg.details ? [msg.details] : (Array.isArray(msg.permitted) ? msg.permitted : (msg.permitted ? [msg.permitted] : []))); + // 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) }); + } else if (has("owner", "spender", "value", "deadline")) { + // EIP-2612. + items.push({ token: td.domain?.verifyingContract, amount: msg.value }); + rows.push({ label: "Valid until", value: when(msg.deadline) }); + } else if (has("details", "spender", "sigDeadline")) { + // Permit2 PermitSingle / PermitBatch. + const list = Array.isArray(msg.details) ? msg.details : [msg.details]; for (const d of list) items.push({ token: d?.token, amount: d?.amount, expiration: d?.expiration }); - if (msg.sigDeadline != null || msg.deadline != null) rows.push({ label: "Signature valid until", value: when(msg.sigDeadline ?? msg.deadline) }); + rows.push({ label: "Signature valid until", value: when(msg.sigDeadline) }); + } else if (has("permitted", "spender", "deadline")) { + // Permit2 PermitTransferFrom / PermitBatchTransferFrom (and *Witness*). + const list = Array.isArray(msg.permitted) ? msg.permitted : [msg.permitted]; + for (const d of list) items.push({ token: d?.token, amount: d?.amount }); + rows.push({ label: "Signature valid until", value: when(msg.deadline) }); } items.slice(0, 10).forEach((it, i) => { const b = big(it.amount); - if (b !== null && b >= UNLIMITED) risky = true; + if (b === null || b >= HUGE) risky = true; rows.push({ label: items.length > 1 ? `Token #${i + 1}` : "Token", value: `${it.token || "?"} — ${amountText(it.amount)}${it.expiration != null ? ` · allowance ${when(it.expiration)}` : ""}`, @@ -3203,7 +3251,7 @@ function describePermit(td) { }); }); if (items.length > 10) { rows.push({ label: "Tokens", value: `… and ${items.length - 10} more` }); risky = true; } - if (!items.length) { rows.push({ label: "Token", value: "(could not read the approval — treat as unlimited)" }); risky = true; } + if (!items.length) { rows.push({ label: "Token", value: "(not a permit shape Aegis can read — treat as unlimited)" }); risky = true; } return { rows, risky }; } @@ -3708,7 +3756,9 @@ function registerPageMessages(api) { } } const domainSummary = [dom.name, dom.version && `v${dom.version}`, dom.chainId && `chain ${dom.chainId}`].filter(Boolean).join(" · ") || "(no domain)"; - const messagePreview = JSON.stringify(td.message, (_k, v) => (typeof v === "bigint" ? v.toString() : v), 2) || ""; + // Preview only what the signature covers (see signedView). + const view = signedView(td); + const messagePreview = JSON.stringify(view.message, (_k, v) => (typeof v === "bigint" ? v.toString() : v), 2) || ""; const rows = [{ label: "Domain", value: domainSummary }]; if (dom.verifyingContract) rows.push({ label: "Contract", value: String(dom.verifyingContract), mono: true }); rows.push({ label: "Primary type", value: String(td.primaryType || "") }); @@ -3718,9 +3768,14 @@ function registerPageMessages(api) { // and the deadline out of the message and say what they mean. const permit = describePermit(td); if (permit) for (const r of permit.rows) rows.push(r); + // Marketplace orders and multisig transactions are not permits but move + // NFTs, tokens or a whole Safe just as surely once signed. + const MOVES_ASSETS = /^(OrderComponents|BulkOrder|Order|MakerOrder|TakerOrder|SafeTx|SafeMessage|MetaTransaction|ForwardRequest)$/; + const movesAssets = !permit && MOVES_ASSETS.test(String(td.primaryType || "")); rows.push({ label: "Message", value: previewText(messagePreview, 3000), mono: true }); + if (view.dropped) rows.push({ label: "Not signed", value: `${view.dropped} field${view.dropped === 1 ? "" : "s"} the site sent are not covered by this signature and are not shown` }); rows.push({ label: "Address", value: snapTd.address, mono: true }); - const risky = !!(permit && permit.risky); + const risky = !!(permit && permit.risky) || movesAssets; return withOriginLock(origin, async () => { const pick = await api.approvalModal({ title: permit ? "Sign a token spending permit?" : "Sign typed data (EIP-712)?", @@ -3729,7 +3784,9 @@ function registerPageMessages(api) { ? (risky ? "WARNING: signing this lets the spender below take these tokens at any time, without another prompt and without a transaction from you. Only sign if you fully trust this site." : "Signing this lets the spender below move the stated amount of your tokens without a transaction from you.") - : "The site is asking you to sign a structured message. Verify the domain matches the site you're on — a mismatched domain is the classic phishing tell.", + : movesAssets + ? `WARNING: a signed ${String(td.primaryType)} can move your NFTs, tokens or Safe without another prompt. Only sign if you fully trust this site and the details below are what you expect.` + : "The site is asking you to sign a structured message. Verify the domain matches the site you're on — a mismatched domain is the classic phishing tell.", rows, actions: [{ id: "sign", label: risky ? "Sign anyway" : "Sign", primary: !risky, danger: risky }], }); diff --git a/bundled-addons/aegis/lib/eip712.js b/bundled-addons/aegis/lib/eip712.js index a9266795..0280e37e 100644 --- a/bundled-addons/aegis/lib/eip712.js +++ b/bundled-addons/aegis/lib/eip712.js @@ -87,8 +87,11 @@ module.exports = function makeEip712({ keccak_256 }) { return keccak_256(b); } if (type === "address") { - const h = hex2bytes(String(value || "0x0").replace(/^0x/, "")); - if (h.length !== 20) throw new Error("address must be 20 bytes"); + // Validate before decoding: the hex decoder turns non-hex characters + // into zero bytes, so a malformed address would sign as a different one. + const s = String(value ?? "").replace(/^0x/i, ""); + if (!/^[0-9a-fA-F]{40}$/.test(s)) throw new Error("address must be 20 bytes of hex"); + const h = hex2bytes(s); const out = new Uint8Array(32); out.set(h, 12); return out;