fix(cards): harden spend refunds, token and destination checks

cards/src/core/CardSpend.sol

@@ -17,7 +17,8 @@ struct CardSpend {
     uint256 destinationHint;
     /// @dev Tempo Earn vault that funds `fromVault`, holding `token`. Zero when `fromVault` is zero.
     address vault;
-    /// @dev Part of `amount` redeemed from `vault`. The rest is paid in `token`.
+    /// @dev Part of `amount` redeemed from `vault`. The rest is paid in `token`. Only a public spend mixes the two: a
+    ///      private spend carries one token, so `fromVault` is zero or all of `amount`.
     uint256 fromVault;
     /// @dev Most EarnShares `fromVault` may burn. Zero accepts the live price.
     uint256 maxShares;

cards/src/core/CardSpender.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.36;
 import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 import { IERC20Like } from "tempo-earn/interfaces/external/IERC20Like.sol";
+import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol";
 import { ADDRESS_REGISTRY_ADDRESS, IAddressRegistry } from "zone-router/interfaces/IAddressRegistry.sol";
 import { IERC20Allowance } from "zone-router/interfaces/IERC20Allowance.sol";
 import { ISpender } from "zone-router/interfaces/ISpender.sol";
@@ -55,6 +56,7 @@ contract CardSpender is ISpender {
     error ResidualBalance(address token);
     error ShareCostExceeded(uint256 required, uint256 maxShares);
     error UnresolvedRecipient();
+    error UnsupportedToken(address token);
     error VaultNotAllowed(address vault);
     error WrongInputToken(address token);
     error WrongProgram(address program);
@@ -224,6 +226,9 @@ contract CardSpender is ISpender {
     function _check(CardSpend memory s, address payer, bool isPrivate) private view returns (address payee) {
         if (s.program != address(this)) revert WrongProgram(s.program);
         if (s.amount == 0 || s.fromVault > s.amount) revert InvalidAmount();
+        // Only TIP-20s settle: no transfer callbacks, fees or rebases to upset the exact-credit checks. A vault's
+        // asset must equal `s.token`, so this covers redemptions too.
+        if (!StdPrecompiles.TIP20_FACTORY.isTIP20(s.token)) revert UnsupportedToken(s.token);
         payee = _payee(s.destination);
         // The program's own balance is the baseline it measures input against, so it never pays itself.
         if (payee == address(0) || payee == address(this)) revert DestinationNotAuthorized(s.destination);
@@ -254,9 +259,15 @@ contract CardSpender is ISpender {
         uint256 required = v.previewWithdraw(s.fromVault);
         if (s.maxShares != 0 && required > s.maxShares) revert ShareCostExceeded(required, s.maxShares);
         address share = v.earnShare();
+        uint256 shareBase = IERC20Like(share).balanceOf(address(this));
         SafeTransferLib.safeTransferFrom(share, msg.sender, address(this), required);
-        sharesUsed = _redeem(v, share, s, payee, required);
-        if (sharesUsed < required) _refund(share, required - sharesUsed);
+        _redeem(v, share, s, payee, required);
+        // The burn is measured, not taken from the vault's report, and may not reach the program's seed.
+        uint256 shareHeld = IERC20Like(share).balanceOf(address(this));
+        if (shareHeld < shareBase) revert ShareCostExceeded(required + shareBase - shareHeld, required);
+        uint256 unused = shareHeld - shareBase;
+        sharesUsed = required - unused;
+        if (unused != 0) _refund(share, unused);
     }
 
     /// @dev A direct private spend, from the `maxInput` the router sent. The vault prices shares itself and

cards/src/providers/bridge/SpenderTempoSpend.sol

@@ -7,6 +7,7 @@ import { SpenderBase } from "bridge-spender/src/spenders/SpenderBase.sol";
 import { IERC20 } from "openzeppelin-contracts/token/ERC20/IERC20.sol";
 import { LibClone } from "solady/utils/LibClone.sol";
 import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
+import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol";
 import { ADDRESS_REGISTRY_ADDRESS, IAddressRegistry } from "zone-router/interfaces/IAddressRegistry.sol";
 
 import { ITempoEarnVault } from "../../core/interfaces/ITempoEarnVault.sol";
@@ -52,9 +53,12 @@ contract SpenderTempoSpend is SpenderBase {
     error DestinationNotAuthorized(address destination);
     error ExpectedBooleanByte();
     error IncorrectAssetForVault();
+    error InvalidVault();
     error ShareCostExceeded(uint256 required, uint256 maxShares);
     error PipelineDisabled();
+    error RefundNotCredited();
     error UnresolvedRecipient();
+    error UnsupportedToken(address token);
     error VaultNotAllowed();
     error WrongProgram(bytes32 spenderId);
 
@@ -116,6 +120,7 @@ contract SpenderTempoSpend is SpenderBase {
         require(data.length == 23, DataIncorrectLength());
         require(data[2] == 0x00 || data[2] == 0x01, ExpectedBooleanByte());
         address vault = address(bytes20(data[3:23]));
+        require(vault != address(0), InvalidVault());
         bool allowed = data[2] == 0x01;
         _storage().allowedVaults[vault] = allowed;
         // The clone's standing EarnShare approval follows the allowlist: set for an allowed vault, so its
@@ -202,10 +207,14 @@ contract SpenderTempoSpend is SpenderBase {
         // Implied by `_isCanonical` under the vendored factory, which registers an ID once and deploys its clone
         // only in `createSpender`; kept because the factory is upgradeable and could re-point an ID.
         require(factory.getSpenderAddress(s.spenderId) == address(this), WrongProgram(s.spenderId));
+        // The literal destination, not the master it resolves to: a master may hand out its virtual addresses as
+        // separate deposit accounts, so allowing the master must not allow them.
         require(factory.isAuthorizedDestination(s.spenderId, s.destination), DestinationNotAuthorized(s.destination));
         require(s.amount > 0 && s.fromVault <= s.amount, InvalidAmount());
         address payee = _payee(s.destination);
         require(payee != address(0) && payee != address(this), DestinationNotAuthorized(s.destination));
+        // Only TIP-20s settle: no transfer callbacks, fees or rebases to upset the exact-credit checks.
+        require(StdPrecompiles.TIP20_FACTORY.isTIP20(s.token), UnsupportedToken(s.token));
 
         uint256 fromStable = s.amount - s.fromVault;
         uint256 before = IERC20(s.token).balanceOf(payee);
@@ -235,6 +244,7 @@ contract SpenderTempoSpend is SpenderBase {
         require(s.maxShares == 0 || required <= s.maxShares, ShareCostExceeded(required, s.maxShares));
 
         address share = v.earnShare();
+        uint256 shareBase = IERC20(share).balanceOf(address(this));
         SafeTransferLib.safeTransferFrom(share, msg.sender, address(this), required);
         // A standing maximum approval, set when governance allowed the vault and topped up here when
         // another handler (such as TF, which clears its own) has lowered it. Clearing it after each spend
@@ -251,14 +261,27 @@ contract SpenderTempoSpend is SpenderBase {
             }
         }
         if (payee == s.destination) {
-            burned = v.withdrawExact(s.fromVault, s.destination, required);
+            v.withdrawExact(s.fromVault, s.destination, required);
         } else {
             uint256 held = IERC20(s.token).balanceOf(address(this));
-            burned = v.withdrawExact(s.fromVault, address(this), required);
+            v.withdrawExact(s.fromVault, address(this), required);
             require(IERC20(s.token).balanceOf(address(this)) == held + s.fromVault, DestinationNotCredited());
             SafeTransferLib.safeTransfer(s.token, s.destination, s.fromVault);
         }
-        if (burned < required) SafeTransferLib.safeTransfer(share, msg.sender, required - burned);
+        // The burn is measured, not taken from the vault's report, and may not reach shares the clone held before.
+        uint256 shareHeld = IERC20(share).balanceOf(address(this));
+        require(shareHeld >= shareBase, ShareCostExceeded(required + shareBase - shareHeld, required));
+        uint256 unused = shareHeld - shareBase;
+        burned = required - unused;
+        if (unused != 0) _refund(share, unused);
+    }
+
+    /// @dev Returns unused shares to the payer. Receive policies can redirect a successful transfer to a guard
+    ///      (TIP-1028), so the payer's balance must rise by exactly `amount`.
+    function _refund(address share, uint256 amount) private {
+        uint256 before = IERC20(share).balanceOf(msg.sender);
+        SafeTransferLib.safeTransfer(share, msg.sender, amount);
+        require(IERC20(share).balanceOf(msg.sender) == before + amount, RefundNotCredited());
     }
 
     /// @dev Who a transfer to `to` credits: its TIP-1022 master if it is a virtual address, else `to`.

cards/test/bridge/unit/BridgeProgram.t.sol

@@ -9,6 +9,7 @@ import {
     ZONE_FACTORY_ADDRESS as ROUTER_ZONE_FACTORY,
     ZoneInfo as RouterZoneInfo
 } from "tempo-earn/interfaces/external/tempo/IZone.sol";
+import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol";
 import { ZoneRouter } from "zone-router/ZoneRouter.sol";
 import { ZoneRouterFactory } from "zone-router/ZoneRouterFactory.sol";
 
@@ -378,6 +379,12 @@ contract BridgeProgramTest is AccessKeys {
     ///         reentrancy lock, and the spend still settles.
     function test_rejectsReentrancy() public {
         ReentrantToken token = new ReentrantToken();
+        // Stands in for a TIP-20, so the program's own lock is what stops the callback.
+        vm.mockCall(
+            StdPrecompiles.TIP20_FACTORY_ADDRESS,
+            abi.encodeWithSignature("isTIP20(address)", address(token)),
+            abi.encode(true)
+        );
         token.mint(router, AMOUNT);
         token.arm(address(program), abi.encodeCall(CardSpender.spendFrom, (address(token), 1, "")));
         CardSpend memory spend = _stableSpend(AMOUNT);

cards/test/bridge/unit/BridgeProviderMock.t.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.36;
 
 import { Test } from "forge-std/Test.sol";
 import { IERC165 } from "openzeppelin-contracts/utils/introspection/IERC165.sol";
+import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol";
 
 import { CardSpend } from "../../../src/core/CardSpend.sol";
 import { CardSpender } from "../../../src/core/CardSpender.sol";
@@ -185,6 +186,17 @@ contract BridgeProviderMockTest is Test {
     function setUp() public {
         usd = new SpenderMockToken();
         share = new SpenderMockToken();
+        // The mock tokens stand in for TIP-20s, which the program requires.
+        vm.mockCall(
+            StdPrecompiles.TIP20_FACTORY_ADDRESS,
+            abi.encodeWithSignature("isTIP20(address)", address(usd)),
+            abi.encode(true)
+        );
+        vm.mockCall(
+            StdPrecompiles.TIP20_FACTORY_ADDRESS,
+            abi.encodeWithSignature("isTIP20(address)", address(share)),
+            abi.encode(true)
+        );
         vault = new MockVault(address(share), address(usd));
         clone = new MockClone();
         clone.setVaultAnswer(abi.encode(abi.encode(true)), false);

cards/test/bridge/unit/SpenderTempoSpend.t.sol

@@ -5,6 +5,7 @@ import { SpenderBase } from "bridge-spender/src/spenders/SpenderBase.sol";
 import { IAccessControl } from "openzeppelin-contracts/access/IAccessControl.sol";
 import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 import { FeeConfig } from "tempo-earn/interfaces/core/EarnFeeTypes.sol";
+import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol";
 
 import { ITempoEarnVault } from "../../../src/core/interfaces/ITempoEarnVault.sol";
 import {
@@ -108,9 +109,25 @@ contract SpenderTempoSpendTest is TempoEnv {
         _expectRevert(alice, _stable(0), abi.encodeWithSelector(SpenderBase.InvalidAmount.selector));
     }
 
-    /// @notice A token that delivers less than it was asked to cannot settle a spend.
+    /// @notice Only TIP-20s settle, so a token with fees, callbacks or rebases is refused up front.
+    function test_spendRejectsNonTip20Token() public {
+        FeeOnTransferToken token = new FeeOnTransferToken();
+        token.mint(alice, 100e6);
+        vm.prank(alice);
+        token.approve(spender, type(uint256).max);
+        Spend memory s = _stable(10e6);
+        s.token = address(token);
+        _expectRevert(alice, s, abi.encodeWithSelector(SpenderTempoSpend.UnsupportedToken.selector, address(token)));
+    }
+
+    /// @notice A token that delivers less than it was asked to cannot settle a spend, even past the TIP-20 check.
     function test_spendRejectsFeeOnTransferToken() public {
         FeeOnTransferToken token = new FeeOnTransferToken();
+        vm.mockCall(
+            StdPrecompiles.TIP20_FACTORY_ADDRESS,
+            abi.encodeWithSignature("isTIP20(address)", address(token)),
+            abi.encode(true)
+        );
         token.mint(alice, 100e6);
         vm.prank(alice);
         token.approve(spender, type(uint256).max);
@@ -489,6 +506,13 @@ contract SpenderTempoSpendTest is TempoEnv {
         bridgeFactory.governorCall(SPENDER_ID, abi.encodePacked(PREFIX_TEMPO_SPEND, bytes2("UV"), uint8(1)));
     }
 
+    function test_governorCallRejectsZeroVault() public {
+        vm.prank(bridgeGovernor);
+        vm.expectRevert(SpenderTempoSpend.InvalidVault.selector);
+        bridgeFactory.governorCall(SPENDER_ID, abi.encodePacked(PREFIX_TEMPO_SPEND, bytes2("UV"), uint8(1), address(0)));
+        assertFalse(_vaultAllowed(address(0)));
+    }
+
     function test_governorCallRejectsNonBooleanFlag() public {
         vm.prank(bridgeGovernor);
         vm.expectRevert(SpenderTempoSpend.ExpectedBooleanByte.selector);
@@ -541,6 +565,11 @@ contract SpenderTempoSpendTest is TempoEnv {
             ADDRESS_REGISTRY_ADDRESS, abi.encodeCall(IAddressRegistry.resolveRecipient, (virtualTo)), abi.encode(master)
         );
         VirtualAwareToken vusd = new VirtualAwareToken("VUSD");
+        vm.mockCall(
+            StdPrecompiles.TIP20_FACTORY_ADDRESS,
+            abi.encodeWithSignature("isTIP20(address)", address(vusd)),
+            abi.encode(true)
+        );
         LiteralReceiverVault vault = new LiteralReceiverVault(address(vusd));
         VirtualAwareToken share = VirtualAwareToken(vault.earnShare());
         vusd.mint(address(vault), 100e6);
@@ -567,6 +596,42 @@ contract SpenderTempoSpendTest is TempoEnv {
         assertEq(share.balanceOf(spender), 0);
     }
 
+    /// @notice Allowing a master does not allow its virtual addresses: a master may hand them out as separate
+    ///         deposit accounts, so only the literal destination on the allowlist is paid.
+    function test_spendToAVirtualDestinationOfAnAllowedMasterReverts() public {
+        address virtualTo = address(uint160(0x12345678fdfDfdFdfDFdfdfdFDfD000000000002));
+        address master = makeAddr("virtual.allowed-master");
+        vm.mockCall(
+            ADDRESS_REGISTRY_ADDRESS, abi.encodeCall(IAddressRegistry.resolveRecipient, (virtualTo)), abi.encode(master)
+        );
+        vm.prank(bridgeGovernor);
+        bridgeFactory.updateSpenderDestination(SPENDER_ID, master, true);
+        Spend memory s = _stable(1e6);
+        s.destination = virtualTo;
+        _expectRevert(alice, s, abi.encodeWithSelector(SpenderTempoSpend.DestinationNotAuthorized.selector, virtualTo));
+    }
+
+    /// @notice A vault that burns more shares than it reports cannot make the clone refund shares it held before:
+    ///         the refund and the reported burn are measured from the clone's balance.
+    function test_spendMeasuresTheBurnNotTheVaultReport() public {
+        UnderReportingEarnVault vault = new UnderReportingEarnVault(address(usd));
+        _mintUsd(address(vault), 100e6);
+        _setVault(address(vault), true);
+        MockToken share = vault.SHARE();
+        share.mint(alice, 100e6);
+        share.mint(spender, 5e6);
+        vm.prank(alice);
+        share.approve(spender, type(uint256).max);
+
+        Spend memory s = _earn(10e6, 10e6, 0);
+        s.vault = address(vault);
+        vm.expectEmit(true, true, true, true, spender);
+        emit SpenderTempoSpend.TempoSpendExecuted(s.ref, alice, settlement, address(usd), 10e6, 0, 10e6, 20e6);
+        _spendAs(alice, s);
+        assertEq(share.balanceOf(alice), 80e6, "payer charged the quote and refunded nothing");
+        assertEq(share.balanceOf(spender), 5e6, "the clone's own shares untouched");
+    }
+
     // ------------------------------------------------------------------
     // Helpers
     // ------------------------------------------------------------------
@@ -620,3 +685,38 @@ contract SpenderTempoSpendTest is TempoEnv {
         _cloneCall(_psCall(s));
     }
 }
+
+/// @notice Burns the full quote but reports half of it burnt.
+contract UnderReportingEarnVault is ITempoEarnVault {
+    address public immutable override asset;
+    MockToken public immutable SHARE;
+
+    constructor(address asset_) {
+        asset = asset_;
+        SHARE = new MockToken("Under-reported EarnShare", "uES");
+    }
+
+    function earnShare() external view override returns (address) {
+        return address(SHARE);
+    }
+
+    function previewWithdraw(uint256 assets) public pure override returns (uint256) {
+        return assets * 2;
+    }
+
+    function previewRedeem(uint256 earnShares) external pure override returns (uint256) {
+        return earnShares / 2;
+    }
+
+    function withdrawExact(uint256 assets, address receiver, uint256 maxEarnShares)
+        external
+        override
+        returns (uint256)
+    {
+        uint256 cost = previewWithdraw(assets);
+        require(cost <= maxEarnShares, "max shares");
+        SHARE.transferFrom(msg.sender, address(0xdead), cost);
+        SafeTransferLib.safeTransfer(asset, receiver, assets);
+        return cost / 2;
+    }
+}