diff --git a/.changeset/harden-solana-swap-signing.md b/.changeset/harden-solana-swap-signing.md new file mode 100644 index 00000000..4b6c5f1b --- /dev/null +++ b/.changeset/harden-solana-swap-signing.md @@ -0,0 +1,5 @@ +--- +"nansen-cli": minor +--- + +Validate Solana swap quotes against the original request before signing (local, Privy, and WalletConnect wallets). The CLI now checks that a quote's chain, token pair, amounts, and target wallet match what was requested at quote time and refuses to sign when they don't, bringing Solana in line with the existing EVM checks. `--swap-mode exactOut` now also requires `--max-input` on Solana (previously EVM-only), so the maximum spend is bounded by a value you supply rather than one taken from the quote itself. diff --git a/src/__tests__/trade-validation.test.js b/src/__tests__/trade-validation.test.js index 83daabb9..1d3acdc7 100644 --- a/src/__tests__/trade-validation.test.js +++ b/src/__tests__/trade-validation.test.js @@ -1219,6 +1219,26 @@ describe('assertQuoteMatchesRequest', () => { expect(() => assertQuoteMatchesRequest(request, mixed, { chain: 'base' })).not.toThrow(); }); + it('treats both Solana native-SOL sentinels as the same asset (Jupiter wrapped mint vs. Relay/OKX system-program ID)', () => { + // resolveTokenAddress persists the wrapped-SOL mint into the request, but + // some aggregators (Relay, OKX) report the System Program ID as inputMint/ + // outputMint for native SOL. Both denote native SOL, so neither direction + // of this pairing may false-reject a benign quote. + const SOL_WRAPPED = 'So11111111111111111111111111111111111111112'; + const SOL_SYSTEM = '11111111111111111111111111111111'; + const solRequest = { + chain: 'solana', walletAddress: 'Wallet1111111111111111111111111111111111', + fromToken: SOL_WRAPPED, toToken: 'EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', + swapMode: 'exactIn', amount: '1000000000', maxInputAmount: '1000000000', + }; + const okxQuote = { inputMint: SOL_SYSTEM, outputMint: 'EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', inputAmount: '1000000000', inAmount: '1000000000', outAmount: '50000000' }; + expect(() => assertQuoteMatchesRequest(solRequest, okxQuote, { chain: 'solana' })).not.toThrow(); + + // A genuinely different sell token must still be rejected. + const poisoned = { ...okxQuote, inputMint: 'DifferentMint11111111111111111111111111111' }; + expect(() => assertQuoteMatchesRequest(solRequest, poisoned, { chain: 'solana' })).toThrow(/sell token .* does not match/i); + }); + it('rejects a swapped-in sell token', () => { const poisoned = { ...okQuote, inputMint: USDT }; expect(() => assertQuoteMatchesRequest(request, poisoned, { chain: 'base' })).toThrow(/sell token .* does not match/i); @@ -1383,6 +1403,34 @@ describe('assertInputWithinMax', () => { expect(() => assertInputWithinMax({ maxInputAmount: '1000000' }, {})).toThrow(/missing the input amount/i); expect(() => assertInputWithinMax({ maxInputAmount: '1000000' }, { inAmount: '1.5' })).toThrow(/not an integer/i); }); + + it('uses Solana-specific wording (no EVM approval/native-value language) for a Solana over-cap', () => { + // Solana has no ERC-20 approval step, so the over-cap message must not + // mention "approval" or "native value" — those are EVM-only concepts. + const err = (() => { + try { assertInputWithinMax({ chain: 'solana', swapMode: 'exactOut', maxInputAmount: '999999' }, base, 0); } + catch (e) { return e.message; } + })(); + expect(err).toMatch(/exceeds your maximum input/i); + expect(err).not.toMatch(/approval/i); + // exactIn variant likewise omits the EVM approval/native-value clause. + const errIn = (() => { + try { assertInputWithinMax({ chain: 'solana', swapMode: 'exactIn', maxInputAmount: '999999' }, base, 0); } + catch (e) { return e.message; } + })(); + expect(errIn).not.toMatch(/approval|native value/i); + }); + + it('picks the Solana wording case-insensitively (chain persisted as "Solana")', () => { + // request.chain is stored verbatim from --chain, so a mixed-case value must + // still route to the Solana-worded message, not the EVM one. + const err = (() => { + try { assertInputWithinMax({ chain: 'Solana', swapMode: 'exactIn', maxInputAmount: '999999' }, base, 0); } + catch (e) { return e.message; } + })(); + expect(err).toMatch(/exceeds your maximum input/i); + expect(err).not.toMatch(/approval|native value/i); + }); }); // --------------------------------------------------------------------------- diff --git a/src/__tests__/trading.test.js b/src/__tests__/trading.test.js index b0e6b245..3166ee72 100644 --- a/src/__tests__/trading.test.js +++ b/src/__tests__/trading.test.js @@ -26,6 +26,7 @@ import { approvalAmountForSwap, approvalCapForQuote, assertCompleteEvmRequestIntent, + assertCompleteSolanaRequestIntent, validateSwapTarget, assertUsableSpender, stripLeadingZeros, @@ -79,6 +80,23 @@ function evmIntent({ walletAddress, fromToken = BASE_USDC, toToken = OUT_TOKEN, }; } +const SOL_MINT = 'So11111111111111111111111111111111111111112'; +const SOL_USDC = 'EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v'; + +function solanaIntent({ walletAddress, fromToken = SOL_MINT, toToken = SOL_USDC, amount = '1000000000', maxInputAmount = amount, swapMode = 'exactIn', toChain = null, recipient = null } = {}) { + return { + chain: 'solana', + toChain, + walletAddress, + recipient, + fromToken, + toToken, + swapMode, + amount, + maxInputAmount, + }; +} + beforeEach(() => { originalHome = process.env.HOME; tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'nansen-trading-test-')); @@ -737,6 +755,17 @@ describe('assertCompleteEvmRequestIntent', () => { }); }); +describe('assertCompleteSolanaRequestIntent', () => { + it('rejects missing or incomplete Solana request intent', () => { + expect(() => assertCompleteSolanaRequestIntent(null)).toThrow(/missing request intent/i); + expect(() => assertCompleteSolanaRequestIntent(solanaIntent({ walletAddress: '' }))).toThrow(/walletAddress missing/i); + }); + + it('accepts complete Solana request intent', () => { + expect(() => assertCompleteSolanaRequestIntent(solanaIntent({ walletAddress: 'SolAddr1111111111111111111111111111111111' }))).not.toThrow(); + }); +}); + describe('validateSwapTarget', () => { const USDC = '0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913'; const ROUTER = '0xDef1C0ded9bec7F1a1670819833240f027b25EfF'; @@ -1283,9 +1312,16 @@ describe('WalletConnect execute support', () => { success: true, quotes: [{ aggregator: 'jupiter', + inputMint: SOL_MINT, + outputMint: SOL_USDC, + inAmount: '1000000000', + outAmount: '50000000', transaction: txBase64, }], - }, 'solana', 'walletconnect'); + }, 'solana', 'walletconnect', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: '9WzDXwBbmkg8ZTbNMqUxvQRAyrZzDsGYdLVL9zYtAWWM' }), + }); const logs = []; const cmds = buildTradingCommands({ @@ -1416,10 +1452,20 @@ describe('Privy execute support', () => { transaction: 'AQAAAA==', metadata: { requestId: 'req-123' }, }], - }, 'solana', 'privy', { evm: 'wl_evm_1', solana: 'wl_sol_1' }); + }, 'solana', 'privy', { evm: 'wl_evm_1', solana: 'wl_sol_1' }, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: 'SolPrivyAddr1111111111111111111111111111' }), + }); - vi.stubGlobal('fetch', vi.fn().mockImplementation((url) => { + vi.stubGlobal('fetch', vi.fn().mockImplementation((url, opts) => { const urlStr = typeof url === 'string' ? url : url.toString(); + // Privy getWallet (to resolve address) + if (urlStr.includes('privy.io') && opts?.method === 'GET') { + return Promise.resolve({ + ok: true, + json: () => Promise.resolve({ id: 'wl_sol_1', address: 'SolPrivyAddr1111111111111111111111111111', chain_type: 'solana' }), + }); + } // Privy signSolanaTransaction if (urlStr.includes('privy.io')) { return Promise.resolve({ @@ -1825,7 +1871,7 @@ describe('quote handler rejects decimal amounts before API call', () => { }); }); -describe('exactOut --max-input requirement is EVM-only', () => { +describe('exactOut --max-input requirement applies to every chain', () => { it('base exactOut without --max-input is rejected before any API call', async () => { const origFetch = global.fetch; global.fetch = vi.fn(); @@ -1840,19 +1886,19 @@ describe('exactOut --max-input requirement is EVM-only', () => { global.fetch = origFetch; }); - it('solana exactOut does NOT require --max-input (Solana has no EVM approval to scope)', async () => { - // The spend-ceiling guards live only in the EVM execute paths, so requiring - // the flag on Solana would break existing users for no security gain. The - // quote may still reject at a later stage (wallet/network), but it must not - // reject on the missing --max-input flag the way an EVM exactOut quote does. + it('solana exactOut without --max-input is rejected before any API call', async () => { + // A cap derived from the API's own quote response would just check that + // quote against itself and could never reject anything — --max-input must + // be an independently supplied ceiling, same as EVM. const origFetch = global.fetch; - global.fetch = vi.fn().mockRejectedValue(new Error('network stubbed off')); + global.fetch = vi.fn(); const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); await expect(cmds.quote([], null, {}, { chain: 'solana', from: 'SOL', to: 'USDC', amount: '1000000', 'swap-mode': 'exactOut', - })).rejects.not.toThrow(/requires --max-input/i); + })).rejects.toThrow(/requires --max-input/i); + expect(global.fetch).not.toHaveBeenCalled(); global.fetch = origFetch; }); @@ -2036,6 +2082,7 @@ describe('quote command with --amount-unit token', () => { amount: '1', 'amount-unit': 'token', 'swap-mode': 'exactOut', + 'max-input': '2000000000', }); const quoteCall = fetchCalls.find(c => c.url.includes('quote')); @@ -2179,6 +2226,7 @@ describe('quote command with --amount-unit usd', () => { amount: '50', 'amount-unit': 'usd', 'swap-mode': 'exactOut', + 'max-input': '2000000000', }); // Should have searched for USDC (the --to token), not SOL @@ -2314,6 +2362,7 @@ describe('quote command with --amount-unit usd', () => { amount: '50', 'amount-unit': 'usd', 'swap-mode': 'exactOut', + 'max-input': '2000000000', }); // exactOut should skip balance check and succeed (reach quote API) @@ -3913,7 +3962,17 @@ describe('Relay aggregator: --gasless flag dispatch', () => { steps: [{ kind: 'transaction', items: [{ data: 'opaque-step-blob' }] }], }, }], - }, 'solana', 'local', null, 'base'); + }, 'solana', 'local', null, 'base', { + swapMode: 'exactIn', + request: solanaIntent({ + walletAddress: showWallet('default').solana, + fromToken: '11111111111111111111111111111111', + toToken: '0x833589fcd6edb6e08f4c7c32d4f71b54bda02913', + toChain: 'base', + amount: '1000000000', + maxInputAmount: '1000000000', + }), + }); const logs = []; const cmds = buildTradingCommands({ log: (m) => logs.push(m), exit: () => {} }); @@ -4402,7 +4461,17 @@ describe('Relay aggregator: Solana non-gasless omits requestId', () => { transaction: txBase64, metadata: { requestId: 'relay-sol-req', isCrossChain: true, bridgeTool: 'relay' }, }], - }, 'solana', 'local', null, 'base'); + }, 'solana', 'local', null, 'base', { + swapMode: 'exactIn', + request: solanaIntent({ + walletAddress: showWallet('default').solana, + fromToken: '11111111111111111111111111111111', + toToken: '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee', + toChain: 'base', + amount: '8300000', + maxInputAmount: '8300000', + }), + }); const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); try { await cmds.execute([], null, {}, { quote: quoteId }); } catch { /* bridge poll may fail, ok */ } @@ -4451,7 +4520,10 @@ describe('Relay aggregator: Solana non-gasless omits requestId', () => { transaction: txBase64, metadata: { requestId: 'jupiter-ultra-req', quoteId: 'aggregator-jupiter-quote-id' }, }], - }, 'solana'); + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana }), + }); const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); try { await cmds.execute([], null, {}, { quote: quoteId }); } catch { /* ok */ } @@ -4497,7 +4569,10 @@ describe('Relay aggregator: Solana non-gasless omits requestId', () => { transaction: txBase64, metadata: { requestId: 'jupiter-ultra-req', quoteId: 'aggregator-jupiter-quote-id' }, }], - }, 'solana'); + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana }), + }); const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); try { await cmds.execute([], null, {}, { quote: quoteId }); } catch { /* ok */ } @@ -4941,3 +5016,295 @@ describe('verifySwapOutcome (execute-path wiring)', () => { expect(logs.some((l) => /no request intent/i.test(l))).toBe(true); }); }); + +// =========================================================================== +// Solana intent binding (adversarial) +// +// Solana signs the aggregator's serialized VersionedTransaction verbatim, with +// no approval/calldata split to independently validate — assertQuoteMatchesRequest +// is the only guard. Each test below crafts a quote that a compromised or buggy +// Trading API might return and confirms the execute path refuses to sign it, +// across all three Solana signing paths (local, Privy, WalletConnect). +// =========================================================================== +describe('Solana intent binding (adversarial)', () => { + const BONK = 'DezXAZ8z7PnrnRJjz3wXBoRgixCa6xjnB7YaB1pPB263'; + const stubRpcFetch = (calls) => vi.fn().mockImplementation((url) => { + calls.push(typeof url === 'string' ? url : url.toString()); + return Promise.resolve({ ok: true, text: () => Promise.resolve(JSON.stringify({ jsonrpc: '2.0', id: 1, result: null })) }); + }); + const noBroadcast = (calls) => expect(calls.some(c => c.includes('/execute'))).toBe(false); + + afterEach(() => { vi.unstubAllGlobals(); vi.restoreAllMocks(); delete process.env.NANSEN_WALLET_PASSWORD; delete process.env.PRIVY_APP_ID; delete process.env.PRIVY_APP_SECRET; }); + + it('local wallet: refuses a quote with a tampered token pair', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + const calls = []; + vi.stubGlobal('fetch', stubRpcFetch(calls)); + + const quoteId = saveQuote({ + success: true, + quotes: [{ + aggregator: 'jupiter', + inputMint: 'DifferentMint111111111111111111111111111', // tampered — not the requested SOL_MINT + outputMint: SOL_USDC, + inAmount: '1000000000', outAmount: '50000000', + transaction: 'AA==', + }], + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/sell token .* does not match/i); + noBroadcast(calls); + }); + + it('local wallet: refuses a quote whose output token differs from the request', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + const calls = []; + vi.stubGlobal('fetch', stubRpcFetch(calls)); + + const quoteId = saveQuote({ + success: true, + quotes: [{ + aggregator: 'jupiter', + inputMint: SOL_MINT, + outputMint: BONK, // tampered — request asked for SOL_USDC + inAmount: '1000000000', outAmount: '50000000', + transaction: 'AA==', + }], + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana }), // toToken defaults to SOL_USDC + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/buy token .* does not match the requested token/i); + noBroadcast(calls); + }); + + it('local wallet: refuses a quote whose input is inflated beyond the requested amount', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + const calls = []; + vi.stubGlobal('fetch', stubRpcFetch(calls)); + + const quoteId = saveQuote({ + success: true, + quotes: [{ + aggregator: 'jupiter', + inputMint: SOL_MINT, outputMint: SOL_USDC, + inAmount: '5000000000', // 5x the requested amount + outAmount: '50000000', + transaction: 'AA==', + }], + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana, amount: '1000000000', maxInputAmount: '1000000000' }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/does not match the requested input/i); + noBroadcast(calls); + }); + + it('local wallet: refuses to sign a quote built for a different wallet', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + const calls = []; + vi.stubGlobal('fetch', stubRpcFetch(calls)); + + const quoteId = saveQuote({ + success: true, + quotes: [{ + aggregator: 'jupiter', + inputMint: SOL_MINT, outputMint: SOL_USDC, + inAmount: '1000000000', outAmount: '50000000', + transaction: 'AA==', + }], + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + // Fixed address that will not match the freshly generated local wallet — + // as if the default wallet changed since quoting. + request: solanaIntent({ walletAddress: '11111111111111111111111111111111' }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/built for wallet .* but the signer is/i); + noBroadcast(calls); + }); + + it('local wallet: refuses a quote with no persisted request intent', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + const calls = []; + vi.stubGlobal('fetch', stubRpcFetch(calls)); + + // saveQuote with no options — as a pre-intent CLI version, or a corrupted + // quote file, would produce. + const quoteId = saveQuote({ + success: true, + quotes: [{ aggregator: 'jupiter', inputMint: SOL_MINT, outputMint: SOL_USDC, inAmount: '1000000000', outAmount: '50000000', transaction: 'AA==' }], + }, 'solana', 'local'); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/missing request intent/i); + noBroadcast(calls); + }); + + it('Privy: refuses to sign a quote built for a different wallet', async () => { + process.env.PRIVY_APP_ID = 'test-app-id'; + process.env.PRIVY_APP_SECRET = 'test-secret'; + const calls = []; + vi.stubGlobal('fetch', vi.fn().mockImplementation((url, opts) => { + const urlStr = typeof url === 'string' ? url : url.toString(); + calls.push({ url: urlStr, method: opts?.method }); + if (urlStr.includes('privy.io') && opts?.method === 'GET') { + return Promise.resolve({ ok: true, json: () => Promise.resolve({ id: 'wl_sol_1', address: 'ActualPrivySolWalletAddr11111111111111111', chain_type: 'solana' }) }); + } + return Promise.resolve({ ok: true, text: () => Promise.resolve(JSON.stringify({ jsonrpc: '2.0', id: 1, result: null })), json: () => Promise.resolve({}) }); + })); + + const quoteId = saveQuote({ + success: true, + quotes: [{ aggregator: 'jupiter', inputMint: SOL_MINT, outputMint: SOL_USDC, inAmount: '1000000000', outAmount: '50000000', transaction: 'AA==' }], + }, 'solana', 'privy', { evm: 'wl_evm_1', solana: 'wl_sol_1' }, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: 'RequestBuiltForADifferentWallet111111111' }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/built for wallet .* but the signer is/i); + // Never reached signSolanaTransaction (the only privy.io POST on this path). + expect(calls.some(c => c.url.includes('privy.io') && c.method === 'POST')).toBe(false); + expect(calls.some(c => c.url.includes('/execute'))).toBe(false); + }); + + it('Privy: refuses a quote whose input is inflated above the request', async () => { + process.env.PRIVY_APP_ID = 'test-app-id'; + process.env.PRIVY_APP_SECRET = 'test-secret'; + const calls = []; + vi.stubGlobal('fetch', vi.fn().mockImplementation((url, opts) => { + const urlStr = typeof url === 'string' ? url : url.toString(); + calls.push({ url: urlStr, method: opts?.method }); + if (urlStr.includes('privy.io') && opts?.method === 'GET') { + return Promise.resolve({ ok: true, json: () => Promise.resolve({ id: 'wl_sol_1', address: 'SolPrivyAddr1111111111111111111111111111', chain_type: 'solana' }) }); + } + return Promise.resolve({ ok: true, text: () => Promise.resolve(JSON.stringify({ jsonrpc: '2.0', id: 1, result: null })), json: () => Promise.resolve({}) }); + })); + + const quoteId = saveQuote({ + success: true, + quotes: [{ + aggregator: 'jupiter', + inputMint: SOL_MINT, outputMint: SOL_USDC, + inAmount: '2000000000', // poisoned: request bound the input to 1,000,000,000 + outAmount: '50000000', + transaction: 'AA==', + }], + }, 'solana', 'privy', { evm: 'wl_evm_1', solana: 'wl_sol_1' }, null, { + swapMode: 'exactIn', + request: solanaIntent({ + walletAddress: 'SolPrivyAddr1111111111111111111111111111', + amount: '1000000000', + maxInputAmount: '1000000000', + }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/does not match the requested input/i); + // Never reached signSolanaTransaction (the only privy.io POST on this path). + expect(calls.some(c => c.url.includes('privy.io') && c.method === 'POST')).toBe(false); + expect(calls.some(c => c.url.includes('/execute'))).toBe(false); + }); + + it('WalletConnect: refuses to sign a quote built for a different wallet', async () => { + vi.spyOn(wcTrading, 'getWalletConnectAddress').mockResolvedValue('ConnectedSolWalletAddr111111111111111111'); + const sendSpy = vi.spyOn(wcTrading, 'sendSolanaTransactionViaWalletConnect').mockResolvedValue({ signedTransaction: '5K4Ld...' }); + + const quoteId = saveQuote({ + success: true, + quotes: [{ aggregator: 'jupiter', inputMint: SOL_MINT, outputMint: SOL_USDC, inAmount: '1000000000', outAmount: '50000000', transaction: 'AA==' }], + }, 'solana', 'walletconnect', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: 'DifferentSolWalletAddr11111111111111111' }), + }); + + const cmds = buildTradingCommands({ log: () => {}, exit: () => {} }); + await expect(cmds.execute([], null, {}, { quote: quoteId })).rejects.toThrow(/built for wallet .* but the signer is/i); + expect(sendSpy).not.toHaveBeenCalled(); + }); + + it('regression: a benign Solana quote still signs and broadcasts', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + + // Minimal valid Solana VersionedTransaction so signSolanaTransaction succeeds. + const sigCount = Buffer.from([0x01]); + const emptySig = Buffer.alloc(64); + const message = Buffer.from([0x01, 0x00, 0x01, 0x02, ...Buffer.alloc(32), ...Buffer.alloc(32), ...Buffer.alloc(32), 0x01, 0x01, 0x01, 0x00, 0x04, 0x02, 0x00, 0x00, 0x00]); + const txBase64 = Buffer.concat([sigCount, emptySig, message]).toString('base64'); + + vi.stubGlobal('fetch', vi.fn().mockImplementation((url) => { + const urlStr = typeof url === 'string' ? url : url.toString(); + if (urlStr.includes('trading-api') && urlStr.endsWith('/execute')) { + return Promise.resolve({ ok: true, text: () => Promise.resolve(JSON.stringify({ status: 'Success', signature: 'SolSig', chainType: 'solana', broadcaster: 'jupiter' })) }); + } + return Promise.resolve({ ok: true, text: () => Promise.resolve(JSON.stringify({ jsonrpc: '2.0', id: 1, result: null })) }); + })); + + const quoteId = saveQuote({ + success: true, + quotes: [{ aggregator: 'jupiter', inputMint: SOL_MINT, outputMint: SOL_USDC, inAmount: '1000000000', outAmount: '50000000', transaction: txBase64 }], + }, 'solana', 'local', null, null, { + swapMode: 'exactIn', + request: solanaIntent({ walletAddress: showWallet('default').solana }), + }); + + const logs = []; + const cmds = buildTradingCommands({ log: (m) => logs.push(m), exit: () => {} }); + await cmds.execute([], null, {}, { quote: quoteId }); + + expect(logs.some(l => l.includes('Transaction successful'))).toBe(true); + }); +}); + +describe('Solana exactOut ceiling — requires an explicit --max-input', () => { + afterEach(() => { delete process.env.NANSEN_WALLET_PASSWORD; }); + + it('persists the explicit --max-input as maxInputAmount', async () => { + createWallet('default', 'testpass'); + process.env.NANSEN_WALLET_PASSWORD = 'testpass'; + + const origFetch = global.fetch; + global.fetch = vi.fn().mockImplementation(async (url) => { + const urlStr = url.toString(); + if (urlStr.includes('/quote')) { + return { + ok: true, + text: async () => JSON.stringify({ + success: true, + quotes: [{ aggregator: 'jupiter', inputMint: SOL_MINT, outputMint: SOL_USDC, inAmount: '1000000000', outAmount: '50000000' }], + }), + }; + } + return { ok: true, text: async () => JSON.stringify({ jsonrpc: '2.0', id: 1, result: null }) }; + }); + + const logs = []; + const cmds = buildTradingCommands({ log: (m) => logs.push(m), exit: () => {} }); + + await cmds.quote([], null, {}, { + chain: 'solana', from: 'SOL', to: 'USDC', amount: '50000000', 'swap-mode': 'exactOut', 'max-input': '2000000000', + }); + + const idLine = logs.find(l => l.includes('Quote ID:')); + const quoteId = idLine.split('Quote ID:')[1].trim(); + expect(loadQuote(quoteId).request.maxInputAmount).toBe('2000000000'); + + global.fetch = origFetch; + }); +}); diff --git a/src/schema.json b/src/schema.json index 2844ef78..999f4336 100644 --- a/src/schema.json +++ b/src/schema.json @@ -1586,7 +1586,7 @@ }, "max-input": { "type": "string", - "description": "exactOut only: hard ceiling on the sell-token spend, in base units. Required for EVM (Base) exactOut and optional on Solana (which has no ERC-20 approval to scope). Measured against the slippage-buffered approval (input + slippage), not the bare quote input, so it matches the amount that can actually leave the wallet. Persisted with the quote and enforced before any approval or signing — a quote whose buffered approval exceeds it is refused." + "description": "exactOut only: hard ceiling on the sell-token spend, in base units. Required on every chain for --swap-mode exactOut. Measured against the slippage-buffered spend (input + slippage), not the bare quote input, so it matches the amount that can actually leave the wallet. Persisted with the quote and enforced before signing — a quote whose buffered spend exceeds it is refused." } }, "prerequisites": [ diff --git a/src/trade-validation.js b/src/trade-validation.js index 3c9de73b..7d01f8d8 100644 --- a/src/trade-validation.js +++ b/src/trade-validation.js @@ -578,7 +578,8 @@ export function needsAllowanceRevoke(existingAllowance, approveAmt) { /** * Compare two token addresses for equality (case-insensitive on EVM, exact on - * Solana). Missing values never match. + * Solana except for the native-SOL sentinel aliasing above). Missing values + * never match. */ function tokensEqual(a, b, chain) { if (!a || !b) return false; @@ -742,13 +743,15 @@ export function assertQuoteMatchesRequest(request, quote, { chain, walletAddress * exactOut gap where the API chooses the input and nothing capped it. * * The amount compared against the cap is the maximum that can actually leave the - * wallet — for exactOut that is the slippage-buffered approval, NOT the raw quote - * input. The approval encoder (encodeApproveCalldata) scopes the ERC-20 approval - * to that same buffered amount and caps it at maxInputAmount, so validating the - * raw input here would let a quote pass this check and then be refused at signing - * (a 1,000,000 input at 3% slippage needs a 1,030,000 approval, which a 1,000,000 - * cap rejects). Comparing the same amount approvalAmountForSwap produces keeps - * this check and the encoder in lockstep. + * wallet — for exactOut that is the slippage-buffered spend, NOT the raw quote + * input. On EVM the approval encoder (encodeApproveCalldata) scopes the ERC-20 + * approval to that same buffered amount and caps it at maxInputAmount, so + * validating the raw input here would let a quote pass this check and then be + * refused at signing (a 1,000,000 input at 3% slippage needs a 1,030,000 + * approval, which a 1,000,000 cap rejects). On Solana there is no approval step, + * but the swap can still consume up to that buffered amount, so the same ceiling + * applies. Comparing the amount approvalAmountForSwap produces keeps this check + * consistent with what the execute path can actually spend. * * Behaviour: * - exactOut with no persisted `maxInputAmount` → throws (fail closed). The @@ -759,8 +762,9 @@ export function assertQuoteMatchesRequest(request, quote, { chain, walletAddress * more than the user approved leave the wallet. * - exactIn with no cap → no-op (request.amount already binds the input). * - * Applies to native and ERC-20 swaps alike; the caller runs it before any - * approval, transaction signing, or WalletConnect call. + * Applies to native, ERC-20, and Solana swaps alike (Solana has no approval step, + * so the "buffered spend" ceiling is just the spend itself); the caller runs it + * before any approval, transaction signing, or WalletConnect call. * * @param {object} request - Persisted intent (quoteData.request) * @param {object} quote - The quote being executed @@ -816,10 +820,18 @@ export function assertInputWithinMax(request, quote, slippage) { ); } if (spend > cap) { + // Normalize case: request.chain is persisted verbatim from the user's + // --chain input (e.g. `--chain Solana`), so an exact === would mislabel a + // Solana swap with the EVM-worded (approval/native-value) message. + const isSolana = String(request.chain).toLowerCase() === 'solana'; throw new Error( swapMode === 'exactOut' - ? `Quote needs an approval of ${spend} base units (input ${input} + slippage buffer) to guarantee the exact output, which exceeds your maximum input (${cap}). Raise --max-input or lower the requested output. Refusing to sign.` - : `Quote input amount (${input}) exceeds your maximum input (${cap}). A larger input would enlarge the approval and native value beyond what you approved. Refusing to sign.`, + ? isSolana + ? `Quote needs ${spend} base units (input ${input} + slippage buffer) to guarantee the exact output, which exceeds your maximum input (${cap}). Raise --max-input or lower the requested output. Refusing to sign.` + : `Quote needs an approval of ${spend} base units (input ${input} + slippage buffer) to guarantee the exact output, which exceeds your maximum input (${cap}). Raise --max-input or lower the requested output. Refusing to sign.` + : isSolana + ? `Quote input amount (${input}) exceeds your maximum input (${cap}). Refusing to sign.` + : `Quote input amount (${input}) exceeds your maximum input (${cap}). A larger input would enlarge the approval and native value beyond what you approved. Refusing to sign.`, ); } } diff --git a/src/trading.js b/src/trading.js index 3506462a..11ac1ce5 100644 --- a/src/trading.js +++ b/src/trading.js @@ -984,6 +984,28 @@ export function assertCompleteEvmRequestIntent(request) { } } +/** + * The Solana sibling of assertCompleteEvmRequestIntent. Solana signs the + * aggregator's serialized VersionedTransaction verbatim — there is no + * approval/calldata split to independently validate — so assertQuoteMatchesRequest + * is the only guard between a compromised quote and a signed drain. That check's + * per-field `if (request.x)` comparisons silently skip a missing field, so this + * closes the gap by failing closed on any incomplete request intent up front. + */ +export function assertCompleteSolanaRequestIntent(request) { + if (!request) { + throw new Error('Quote is missing request intent. Re-quote with this CLI version before executing a Solana swap. Refusing to sign.'); + } + + const missing = []; + for (const field of ['chain', 'walletAddress', 'fromToken', 'toToken', 'swapMode', 'amount', 'maxInputAmount']) { + if (request[field] == null || request[field] === '') missing.push(field); + } + if (missing.length) { + throw new Error(`Quote request intent is incomplete (${missing.join(', ')} missing). Re-quote before executing a Solana swap. Refusing to sign.`); + } +} + /** * Sanity-check the target of a swap transaction before signing it. * @@ -1593,9 +1615,9 @@ OPTIONS: --swap-mode exactIn (default) or exactOut --max-input exactOut only: hard ceiling on the sell-token spend (base units), measured against the slippage-buffered - approval (input + slippage), not the bare quote input. - Required for EVM (Base) exactOut and enforced before - signing; optional on Solana (no ERC-20 approval to scope). + spend (input + slippage), not the bare quote input. + Required for exactOut on every chain and enforced + before signing. --aggregator Force a specific aggregator (lifi, relay, jupiter, okx). Filters the quote list client-side; errors if none match. @@ -1630,12 +1652,9 @@ CROSS-CHAIN NOTES (when using --to-chain): throw new CommandError('Error: --amount-unit percent is not supported with --swap-mode exactOut. Percentage is relative to your sell-token balance.', 'INVALID_INPUT'); } - // The exactOut spend-ceiling requirements below only guard the EVM signing - // path: the ERC-20 approval scoping, request-intent binding, and - // assertInputWithinMax checks are wired into the EVM execute paths only. - // Solana signs the API transaction verbatim (no approval to scope), so - // requiring --max-input there would break existing Solana exactOut users - // without buying any of that path a security guarantee. Gate on EVM source. + // isEvmSource gates the ERC-20-approval-specific check just below (auto-slippage + // sizing an approval has no Solana equivalent). The --max-input requirement + // itself is NOT gated on it — see the check after maxInputOverride is parsed. const isEvmSource = CHAIN_MAP[chain?.toLowerCase()]?.type === 'evm'; // exactOut scopes the ERC-20 approval to a slippage-buffered max input. With @@ -1669,7 +1688,10 @@ CROSS-CHAIN NOTES (when using --to-chain): throw new CommandError(`Error: invalid --max-input "${maxInputRaw}": must be an integer in base units of the sell token.`, 'INVALID_INPUT'); } } - if (isEvmSource && swapMode === 'exactOut' && maxInputOverride == null) { + // Required on every chain: an exactOut cap derived from the API's own quote + // response would just check that quote against itself and could never reject + // anything (there is no independent signal to catch an inflated input). + if (swapMode === 'exactOut' && maxInputOverride == null) { throw new CommandError('Error: --swap-mode exactOut requires --max-input (base units of the sell token) so the input is independently capped before signing.', 'INVALID_INPUT'); } @@ -1922,6 +1944,9 @@ CROSS-CHAIN NOTES (when using --to-chain): } const signerType = isWalletConnect ? 'walletconnect' : walletProvider; + // exactOut has no request.amount input bound (amount is the OUTPUT), so + // maxInputAmount is the only spend ceiling assertInputWithinMax can enforce. + // Required explicitly via --max-input on every chain (checked above). const maxInputAmount = swapMode === 'exactOut' ? maxInputOverride : String(resolvedAmount); const quoteId = saveQuote(response, chain, signerType, privyWalletIds, isCrossChain ? toChainRaw : null, { swapMode, @@ -2141,9 +2166,27 @@ EXAMPLES: if (typeof txBase64 === 'object' && txBase64.data) { txBase64 = base58Decode(txBase64.data).toString('base64'); } - log(' Signing Solana transaction via Privy...'); + const solWalletId = quoteData.privyWalletIds?.solana; if (!solWalletId) throw new Error('No Solana Privy wallet ID in quote'); + const walletResult = await privyClient.getWallet(solWalletId); + const walletAddress = walletResult.address; + // Fail closed if the signer address doesn't resolve: without it the + // wallet-binding comparison below would silently skip, leaving the + // quote unbound to the wallet that will sign it. + if (!walletAddress) { + throw new Error('Could not resolve the Solana Privy wallet address; cannot confirm the quote was built for this wallet. Refusing to sign.'); + } + + // Validate the persisted request/quote metadata (token pair, amounts, + // signer) before signing the aggregator's serialized transaction as + // returned. This does not inspect the serialized transaction's own + // instructions — Solana quotes have no `to`/`data`/approval split to + // check independently, unlike EVM's validateSwapTarget. + assertCompleteSolanaRequestIntent(quoteData.request); + assertQuoteMatchesRequest(quoteData.request, currentQuote, { chain, walletAddress, slippage: quoteData.slippage }); + + log(' Signing Solana transaction via Privy...'); const signResult = await privyClient.signSolanaTransaction(solWalletId, txBase64); signedTransaction = signResult.data?.signed_transaction || signResult.signed_transaction; requestId = currentQuote.metadata?.requestId; @@ -2408,12 +2451,12 @@ EXAMPLES: signedTransaction = signResult.data?.signed_transaction || signResult.signed_transaction; } else if (chainType === 'solana') { - // NB: validateSwapTarget (the EVM `to`/`data` guard) intentionally does - // not apply here — Solana quotes are a pre-built serialized - // VersionedTransaction with no `to`/`data`/approval split to validate, - // and this path (including the WalletConnect sub-branch below) signs it - // as supplied. Deeper Solana inspection (e.g. checking instruction - // program IDs) is tracked as a follow-up, not an oversight. + // NB: validateSwapTarget (the EVM `to`/`data` guard) does not apply + // here — Solana quotes are a pre-built serialized VersionedTransaction + // with no `to`/`data`/approval split to validate. assertQuoteMatchesRequest + // below binds the metadata (token pair, amounts, signer) instead. + // Deeper static inspection of the tx's own instructions is tracked + // as a follow-up, not an oversight. // Solana: transaction is either a base64 string (Jupiter) or an object // with a base58-encoded `data` field (OKX). Normalize to base64. let txBase64 = currentQuote.transaction; @@ -2421,6 +2464,31 @@ EXAMPLES: txBase64 = base58Decode(txBase64.data).toString('base64'); } + // Resolve the signer for this sub-path so the intent-binding check + // below can confirm the quote was built for this exact wallet. + let solanaWalletAddress; + if (isWalletConnect) { + solanaWalletAddress = await getWalletConnectAddress(chainType); + if (!solanaWalletAddress) { + throw new CommandError('WalletConnect session lost during execute. Reconnect with `walletconnect connect` and retry.', 'NO_WALLET'); + } + } else { + solanaWalletAddress = exported.solana.address; + // Fail closed if the signer address doesn't resolve: without it the + // wallet-binding comparison below would silently skip, leaving the + // quote unbound to the wallet that will sign it. + if (!solanaWalletAddress) { + throw new Error("Could not resolve the local wallet's Solana address; cannot confirm the quote was built for this wallet. Refusing to sign."); + } + } + + // Validate the persisted request/quote metadata (token pair, amounts, + // signer) before signing the opaque Solana transaction. This does not + // inspect the serialized transaction's own instructions — see the note + // above where the Solana branch starts. + assertCompleteSolanaRequestIntent(quoteData.request); + assertQuoteMatchesRequest(quoteData.request, currentQuote, { chain, walletAddress: solanaWalletAddress, slippage: quoteData.slippage }); + if (isWalletConnect) { // Solana via WalletConnect: convert base64 → base58 for WC protocol log(' Signing Solana transaction via WalletConnect...');