From c9ebac822ae7df3f1afef44396717b9767b9a405 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 23 Aug 2026 20:23:05 +0200 Subject: [PATCH] harden: state an undetermined swap input token as undetermined, not as ETH (closes #357) tokenInfo(null) yielded "ETH (native)", so a swap whose input token the calldata never named was asserted to the user as ETH. The determination established for the output side in #353 applies unchanged: Currency is a value type over address, so native ETH arrives as the truthy zero-address string and WRAP_ETH sets it explicitly -- null can only mean undetermined. The rule now lives in tokenInfo() itself rather than at each call site, so the redundant output-side guard added by #356 is removed; both sides read one UNNAMED_CURRENCY constant and cannot drift. Verified by execution that a genuine native-ETH input still renders as ETH, including via WRAP_ETH and a V4 zero-address PoolKey, and that the real mainnet output-side fixture is unchanged. --- TODO.md | 10 ++ src/shared/uniswap.js | 61 ++++---- tests/uniswapUndeterminedTokenIn.test.js | 182 +++++++++++++++++++++++ 3 files changed, 227 insertions(+), 26 deletions(-) create mode 100644 tests/uniswapUndeterminedTokenIn.test.js diff --git a/TODO.md b/TODO.md index f985496..6c3a269 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-23: A swap whose input token the calldata never named is said to be + unknown instead of being called ETH + ([#357](https://git.eeqj.de/sneak/AutistMask/issues/357)), the twin on the + input side of [#353](https://git.eeqj.de/sneak/AutistMask/issues/353). A null + `inputToken` rendered as `Token In: ETH (native)` and titled the swap + `Swap ETH -> X`, asserting the user was paying native ETH when nothing in the + calldata said so. The null-means-ETH collapse is now gone from `tokenInfo()` + itself rather than guarded at each call site: null is refused, and native ETH + keeps arriving as the explicit zero address that `WRAP_ETH` and V4's + `Currency.wrap(address(0))` both use. - 2026-08-23: A swap whose output token the calldata never named is said to be unknown instead of being called ETH ([#353](https://git.eeqj.de/sneak/AutistMask/issues/353)). `tokenInfo(null)` diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 9f19a72..0796bec 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -48,13 +48,32 @@ function formatAmount(raw, decimals) { return truncateAmountNeverZero(formatUnits(raw, decimals)); } +// One wording for either side of the screen: a currency the calldata never +// named. It reads as a refusal, the same stance unknownDecimalsAmount() takes +// on a scale — not as a token name, and not as a quantity. +const UNNAMED_CURRENCY = "Unknown (not named in the calldata)"; + // `decimals` is null when nothing knows this token's scale. It is not // defaulted to 18: the swap lines land on the same approval screen as the // ERC-20 line, and a scale guessed there is what showed a 1,000 USDT swap as // 0.000000000001. `sources` is { trackedTokens, wallets }, shaped as they are // on `state`; resolveTokenDecimals() reads the bundled list, then those. +// +// A null `address` means UNDETERMINED — the calldata named no currency for +// that side — and is refused rather than named. It is not native ETH: Uniswap +// V4 spells native ETH as `Currency.wrap(address(0))` (v4-core +// `type Currency is address`), and a Currency is ABI-encoded as a plain +// address word, so every decode site here gets back the truthy string +// "0x0000000000000000000000000000000000000000" for it — never null. WRAP_ETH +// sets that same explicit zero address, and an UNWRAP_WETH output is caught by +// its caller before this is consulted, so nothing that genuinely is ETH +// arrives null. Naming a null ETH states the wrong asset and formats its +// amount at the wrong scale. function tokenInfo(address, sources) { - if (!address || address === "0x0000000000000000000000000000000000000000") { + if (!address) { + return { symbol: null, decimals: null, address: null }; + } + if (address === "0x0000000000000000000000000000000000000000") { return { symbol: "ETH", decimals: 18, address: null }; } const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); @@ -451,26 +470,14 @@ function decode(data, toAddress, sources) { } } - // Resolve token info. - // - // A null `outputToken` means undetermined, not native ETH, so it is - // not handed to tokenInfo() — which maps null to ETH at 18 decimals - // for the input side's benefit. Uniswap V4 spells native ETH as - // `Currency.wrap(address(0))` (v4-core `type Currency is address`), - // and a Currency is ABI-encoded as a plain address word, so every - // decode site here gets back the truthy string - // "0x0000000000000000000000000000000000000000" for it — never null. - // tokenInfo() already names that ETH, and an UNWRAP_WETH output is - // caught above, so nothing that genuinely outputs ETH arrives null. - // Only a step whose output currency did not decode does, and naming - // that ETH states the wrong asset and formats Min. received at the - // wrong scale. + // Resolve token info. A null token on either side means the calldata + // named no currency for it; tokenInfo() refuses rather than calling it + // ETH. UNWRAP_WETH is the one output that is ETH without a currency to + // decode, and it is answered here rather than left to that rule. const inInfo = tokenInfo(inputToken, sources); const outInfo = hasUnwrapWeth ? { symbol: "ETH", decimals: 18, address: null } - : outputToken - ? tokenInfo(outputToken, sources) - : { symbol: null, decimals: null, address: null }; + : tokenInfo(outputToken, sources); const inSymbol = inInfo.symbol; const outSymbol = outInfo.symbol; @@ -500,6 +507,12 @@ function decode(data, toAddress, sources) { }); } else if (inSymbol === "ETH") { details.push({ label: "Token In", value: "ETH (native)" }); + } else { + // Nothing established the input token, so the line says that + // rather than going missing or naming a token by default. Same + // wording as the Token Out refusal below: the two sides of this + // screen must not describe the same condition in two ways. + details.push({ label: "Token In", value: UNNAMED_CURRENCY }); } if (inputAmount !== null && inputAmount !== undefined) { @@ -538,14 +551,10 @@ function decode(data, toAddress, sources) { details.push({ label: "Token Out", value: outSymbol }); } else { // Nothing established the output token, so the line says that - // rather than going missing or naming a token by default. It reads - // as a refusal, the same stance unknownDecimalsAmount() takes on a - // scale, so a Min. received figure below it is never attached to a - // token the calldata did not state. - details.push({ - label: "Token Out", - value: "Unknown (not named in the calldata)", - }); + // rather than going missing or naming a token by default, and a + // Min. received figure below it is never attached to a token the + // calldata did not state. + details.push({ label: "Token Out", value: UNNAMED_CURRENCY }); } if (minOutput !== null && minOutput !== undefined) { diff --git a/tests/uniswapUndeterminedTokenIn.test.js b/tests/uniswapUndeterminedTokenIn.test.js new file mode 100644 index 0000000..ae0061d --- /dev/null +++ b/tests/uniswapUndeterminedTokenIn.test.js @@ -0,0 +1,182 @@ +// What the dApp approval screen says the input token of a swap is when the +// calldata did not name one. +// +// Issue #357, the twin on the input side of +// https://git.eeqj.de/sneak/AutistMask/issues/353: `tokenInfo(null)` answered +// `{symbol: "ETH", decimals: 18}`, so a null `inputToken` rendered as +// `Token In: ETH (native)` and titled the swap `Swap ETH -> X`. An +// undetermined input was therefore asserted to the user as native ETH — the +// same class as https://git.eeqj.de/sneak/AutistMask/issues/340 and +// https://git.eeqj.de/sneak/AutistMask/issues/306, naming the wrong asset +// rather than merely mis-scaling it. +// +// The determination this file pins is the one #353 established, checked here +// on the input side: null is NOT how native ETH arrives. v4-core declares +// `type Currency is address` and wraps `address(0)` for native ETH, and a +// user-defined value type over `address` carries the plain `address` ABI +// encoding, so a native-ETH currency reaches the decoder as the truthy string +// "0x0000000000000000000000000000000000000000". WRAP_ETH sets that same +// explicit zero address. Both halves are asserted below: the zero address +// stays ETH, and null refuses. + +const { AbiCoder, Interface, solidityPacked } = require("ethers"); +const uniswap = require("../src/shared/uniswap"); + +const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af"; +const USER = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; +const ZERO = "0x0000000000000000000000000000000000000000"; +// Both in the bundled list: USDT at 6 decimals, USDC at 6. +const USDT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; +const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; + +// The same wording the output side refuses with — one screen, one vocabulary. +const REFUSAL = "Unknown (not named in the calldata)"; + +const ONE_ETH = 1000000000000000000n; + +const V4_SWAP_EXACT_IN = 0x07; +const V4_SETTLE = 0x0b; +const V4_TAKE = 0x0e; + +const coder = AbiCoder.defaultAbiCoder(); +const routerIface = new Interface([ + "function execute(bytes commands, bytes[] inputs, uint256 deadline)", +]); + +function execute(commands, inputs) { + return routerIface.encodeFunctionData("execute", [ + commands, + inputs, + 9999999999n, + ]); +} + +function v4Input(actions, params) { + return coder.encode( + ["bytes", "bytes[]"], + [new Uint8Array(actions), params], + ); +} + +// IV4Router.ExactInputParams as the decoder reads it: +// (Currency currencyIn, PathKey[] path, uint128 amountIn, uint128 minOut). +function exactInParams(currencyIn, path, amountIn, amountOutMin) { + return coder.encode( + [ + "tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)", + ], + [[currencyIn, path, amountIn, amountOutMin]], + ); +} + +// BALANCE_CHECK_ERC20 (command 0x0e): (address owner, address token, +// uint256 minBalance). It names the output token and nothing about the input. +function balanceCheck(token, minBalance) { + return coder.encode( + ["address", "address", "uint256"], + [USER, token, minBalance], + ); +} + +function detail(data, label) { + const decoded = uniswap.decode(data, ROUTER, {}); + expect(decoded).not.toBeNull(); + return decoded.details.find((d) => d.label === label); +} + +describe("an execute() whose input currency never decoded", () => { + // A V4_SWAP whose sub-action params do not decode — an action encoding + // this decoder does not know — leaves every V4 token null. The + // BALANCE_CHECK still names the output, so the screen has a Min. received + // figure and a Token Out, and previously claimed the user was paying ETH + // for them. + const data = () => + execute(solidityPacked(["uint8", "uint8"], [0x10, 0x0e]), [ + coder.encode(["bytes", "bytes[]"], ["0x07", ["0x"]]), + balanceCheck(USDC, 2000000n), + ]); + + test("says the input token is unknown instead of naming ETH", () => { + expect(detail(data(), "Token In").value).toBe(REFUSAL); + }); + + test("keeps a Token In line, so the screen never omits what is paid", () => { + const tokenIn = detail(data(), "Token In"); + expect(tokenIn).toBeDefined(); + // A refusal is not a token: nothing to link to an explorer. + expect(tokenIn.address).toBeUndefined(); + expect(tokenIn.isToken).toBeUndefined(); + }); + + test("does not name ETH in the swap title either", () => { + expect(uniswap.decode(data(), ROUTER, {}).name).toBe("Uniswap Swap"); + }); +}); + +describe("an execute() that names only an output token", () => { + // A lone BALANCE_CHECK_ERC20: nothing in it says what is being paid. + const data = () => execute("0x0e", [balanceCheck(USDT, 2000000n)]); + + test("refuses the input rather than defaulting it to ETH", () => { + expect(detail(data(), "Token In").value).toBe(REFUSAL); + expect(uniswap.decode(data(), ROUTER, {}).name).toBe("Uniswap Swap"); + }); + + test("still states the output it did establish", () => { + expect(detail(data(), "Token Out").value).toContain("USDT"); + expect(detail(data(), "Min. received").value).toBe("2.0000 USDT"); + }); +}); + +describe("native ETH in, which V4 spells as the zero address", () => { + test("a Currency of address(0) decodes to a truthy address string", () => { + // The fact the whole determination rests on: an absent input currency + // and a native-ETH one are distinguishable here, because the ABI + // decoder never yields null for an address word. + const [currency] = coder.decode( + ["address", "uint256", "bool"], + coder.encode(["address", "uint256", "bool"], [ZERO, ONE_ETH, true]), + ); + expect(currency).toBe(ZERO); + expect(Boolean(currency)).toBe(true); + }); + + test("a V4 SETTLE of the zero address is still ETH at 18 decimals", () => { + const data = execute("0x10", [ + v4Input( + [V4_SETTLE, V4_SWAP_EXACT_IN, V4_TAKE], + [ + coder.encode( + ["address", "uint256", "bool"], + [ZERO, ONE_ETH, true], + ), + exactInParams( + ZERO, + [[USDT, 500, 10, ZERO, "0x"]], + ONE_ETH, + 2000000n, + ), + coder.encode( + ["address", "address", "uint256"], + [USDT, USER, 0n], + ), + ], + ), + ]); + + expect(detail(data, "Token In").value).toBe("ETH (native)"); + expect(detail(data, "Amount").value).toBe("1.0000 ETH"); + expect(uniswap.decode(data, ROUTER, {}).name).toBe("Swap ETH → USDT"); + }); + + test("WRAP_ETH is still ETH at 18 decimals", () => { + // WRAP_ETH names no currency of its own; the decoder supplies the + // explicit zero address for it, which is why it survives this change. + const data = execute("0x0b", [ + coder.encode(["address", "uint256"], [ROUTER, ONE_ETH]), + ]); + + expect(detail(data, "Token In").value).toBe("ETH (native)"); + expect(detail(data, "Amount").value).toBe("1.0000 ETH"); + }); +});