Coinbase: SpendRouter

Description

SpendPermissionManager already supports spendWithWithdraw(), which allows a spender to atomically top up an account from MagicSpend and then spend.

function spendWithWithdraw(        SpendPermission memory spendPermission,        uint160 value,        MagicSpend.WithdrawRequest memory withdrawRequest    ) external requireSender(spendPermission.spender) {        // check spend token and withdraw asset are the same        if (            !(spendPermission.token == NATIVE_TOKEN && withdrawRequest.asset == address(0))                && spendPermission.token != withdrawRequest.asset        ) {            revert SpendTokenWithdrawAssetMismatch(spendPermission.token, withdrawRequest.asset);        }
        // check spend value is not less than withdraw request amount        if (withdrawRequest.amount > value) {            revert SpendValueWithdrawAmountMismatch(value, withdrawRequest.amount);        }
        // check withdraw request nonce postfix matches spend permission hash postfix.        bytes32 permissionHash = getHash(spendPermission);        if (uint128(withdrawRequest.nonce) != uint128(uint256(permissionHash))) {            revert InvalidWithdrawRequestNonce(uint128(withdrawRequest.nonce), uint128(uint256(permissionHash)));        }
        _useSpendPermission(spendPermission, value);        _execute({            account: spendPermission.account,            target: MAGIC_SPEND,            value: 0,            data: abi.encodeWithSelector(MagicSpend.withdraw.selector, withdrawRequest)        });        _transferFrom(spendPermission.token, spendPermission.account, spendPermission.spender, value);    }

However, SpendRouter only wraps spend / approveWithSignature + spend, so flows that rely on MagicSpend cannot preserve the router’s executor / recipient binding in a single transaction. As a result, integrators must either build a custom wrapper or fall back to a two-step flow, losing atomicity and weakening the intended periphery UX.

Recommendation

Add router-level wrappers such as spendWithWithdrawAndRoute() and, if needed, spendWithWithdrawAndRouteWithSignature(), while keeping the existing executor and recipient checks unchanged.

Coinbase

Accepting suggestion. Fix is implemented in https://github.com/coinbase/spend-permissions/pull/81

Cantina

Fixed as recommended.

Description

decodeExtraData() validates that extraData is exactly 64 bytes but does not check that the decoded executor address is non-zero. In spendAndRoute() and spendAndRouteWithSignature(), the only post-decode validation is on recipient and executor is used directly in the msg.sender comparison without a zero-address guard. Since msg.sender can never be address(0) in the EVM, a permission accidentally encoded with a zero executor can never be executed.

While encodeExtraData() already guards against this for users who go through the helper, users who manually craft extraData are susceptible to this scenario.

Recommendation

Consider adding an explicit zero-address check on executor after decoding, either inside decodeExtraData() or at the call sites alongside the existing recipient check.

Coinbase

Acknowledged, won't fix. Since a permission thus constructed poses no security risk, we'll avoid placing the extra check on the happy path.

Cantina

Acknowledged.

Description

The constructor accepts a SpendPermissionManager argument and assigns it directly to the PERMISSION_MANAGER immutable without validating that the address is non-zero. Since PERMISSION_MANAGER is immutable, a deployment with a zero address permanently bricks the contract, with no recovery path short of redeployment. Given that SpendRouter is intended as a singleton, a misconfigured deployment would require redeployment and redistribution of the new address to all dependent systems.

While the deployment script currently deploys SpendPermissionManager immediately before SpendRouter and passes the live address directly, it makes an accidental EOA or undeployed address unlikely in practice. However, a zero-address or a zero-code guard is a defensive check for any misconfiguration risk.

Recommendation

Consider adding an explicit zero-address or zero-code guard in the constructor before assigning the immutable.