Cross-Chain Security Audit of Sprinter Stash

Description

The contract uses the standard approve() function for setting ERC20 token allowances. This approach can cause issues with tokens that require setting the allowance to zero before updating it to a new value (e.g., USDT), potentially resulting in failed transactions.

Recommendation

Use forceApprove().

Description

When triggering Across transfer, all the parameter is decoded from extraData

function initiateTransferAcross(        IERC20 token,        uint256 amount,        address destinationPool,        Domain destinationDomain,        bytes calldata extraData    ) internal {        token.forceApprove(address(ACROSS_SPOKE_POOL), amount);        (            address outputToken, // Can be set to 0x0 for automapping by solvers.            uint256 outputAmount,            address exclusiveRelayer,            uint32 quoteTimestamp, // Validated in the spoke pool            uint32 fillDeadline, // Validated in the spoke pool            uint32 exclusivityDeadline        ) = abi.decode(extraData, (address, uint256, address, uint32, uint32, uint32));        require(outputAmount >= (amount * 9980 / 10000), SlippageTooHigh());        ACROSS_SPOKE_POOL.depositV3(            address(this),            destinationPool,            address(token),            outputToken,            amount,            outputAmount,            domainChainId(destinationDomain),            exclusiveRelayer,            quoteTimestamp,            fillDeadline,            exclusivityDeadline,            "" // message        );    }

https://docs.across.to/introduction/migration-guides/migration-to-cctp/migration-guide-for-relayers

The code enforces a slippage check.

require(outputAmount >= (amount * 9980 / 10000), SlippageTooHigh());

This check works if the input token is the same as the output token.

This check does not work for the two case below:

1. When the input token and output token has different decimals in source chain and destination chain.

In etherscan, the USDT has 6 decimals.

In binance smart chain, the USDT has 18 decimals, then the code cannot use input token amount to validate output token amount.

2. When the input token is not the same as output token.

If user bridge 100 USDC in exchange for WETH in other blockchain.

USDC has 6 decimals.

WETH has 18 decimals.

100 USDC = 10 * 10 ** 6 = 100000000 wei

If the check is applied,

require(outputAmount >= (amount * 9980 / 10000), SlippageTooHigh());

the output WETH amount is 0.0000000001 ETH * 0.98, which is a dust of WETH.

Recommendation

Consider remove the check

require(outputAmount >= (amount * 9980 / 10000), SlippageTooHigh());

Description

contract ManagedToken is IManagedToken, ERC20Permit

The Shares token is a ManagedToken, but the Shares token is always 18 decimals.

However, the asset token and share token can have different decimials.

The Shares token can be transferred or trade.

Smart contracts or UIs expecting 18-decimal precision may perform math incorrectly (e.g., dividing/multiplying by 1e18 instead of 1e6), leading to miscalculations.

Recommendation

Consider make sure the Shares token decimal match asset decimals instead of hardcode the ManagedToken decimal to 18.

Finding Description

depositForBurnWithCaller() function isn't present in CCTP v2 (only in v1). While, there is no indication that v1 is deprecated, it should be explicitly considered which CCTP version should be supported.

Recommendation

v1 and v2 has some differences listed in CCTP docs, and here is the list of supported domains in both versions: domains.

Consider which version to support based on these differences.

Sprinter

Yes, we work with V1 for now.

Cantina

Acknowledged.

Description

The contract uses the value 1000000000 to represent the precision of $10^9$. While this representation is functionally correct, it reduces readability.

Recommendation

Replace the large integer literal with scientific notation 1e9.

Description

The contract uses both msg.sender and _msgSender() inconsistently. While both return the caller currently , overriding _msgSender() will break this compatibility.

Recommendation

Use msg.sender or _msgSender() consistently.

Finding Description

Behavior on different return values of .transferFrom here vs if you used .safeTransferFrom:

• true => succeeds vs succeeds
• no return value => succeeds vs succeeds
• false => succeeds vs reverts

So it differs when the call returns false as the return value.

Recommendation

If compatibility with SafeERC20's behavior is important, consider reverting when the transferFrom() call returns false It can happen with some weird ERC20 token.

Description

IERC20Permit(asset()).permit(            _msgSender(),            address(this),            assets,            deadline,            v,            r,            s        );

if another consume and frontrun the permit signature before depositWithPermit, the depositWithPermit will revert because the signature nonce is already consumed.

Recommendation

Consider try catch the permit external call.

Description

Since the destinationCaller argument in depositForBurnWithCaller() is the address on the source domain, CCTP will only allow that address to process the transfer on the destination domain.

This, it's essential to deploy this adapter on the same address on all domains.

Recommendation

If it's EVM chain, ensure that the adapter is deployed using CREATE2 and that CREATE2 generates the same address (while not known, EVM incompatibility may cause an issue).

Description

The highlighted block can be refactored to check for isIncrease only once.

Recommendation

uint256 newAssetsif (isIncrease) {    require(amount <= _assetsIncreaseHardLimit(assets), AssetsExceedHardLimit());    newAssets = assets + amount;} else {    newAssets = assets - amount;}