The Standard

Description

Some ERC20 tokens are implemented to take a fee on each transfer, e.g., PAXG on Ethereum mainnet. The Arbitrum implementation of PAXG is a ClonableBeaconProxy contract that points to the StandardArbERC20 implementation. While this contract does currently implement any fees on transfer, the ERC20Upgradeable::_transfer NatSpec contains the following comment:

This is internal function is equivalent to {transfer}, and can be used to e.g. implement automatic token fees, slashing mechanisms, etc.

If any of the existing collateral tokens are upgraded to take a fee on transfers or an existing fee-on-transfer token is added to the list of accepted tokens, SmartVaultV3::swap will be affected due to the approval/transfer of tokens in SmartVaultV3::executeERC20SwapAndFee. If the input token implements fees on transfers, then these will be taken when transferring both the protocol fee and input amount that is approved to the router (despite being unaccounted for). While minimumAmoutOut is intended to enforce the minimum output amount required by the protocol, it fails to consider the fee that will be taken when transferring the fee-on-transfer output token from the router to the vault. The swap will succeed, but it is possible that the fee will cause the vault to become liquidatable.

Impact

The slippage parameters that are used in core collateralization validation fail to consider potential value leakage due to fees on token transfers, so this issue is of medium severity.

Proof of Concept

Considering a 2% protocol swap fee and a transfer fee of 2% on PAXG:

1. Alice deposits 1000 EUR in LINK to her vault.

2. Alice mints 200 EURO.

3. Alice attempts to swap 1000 EUR in LINK for PAXG.

4. The swap fee is calculated as 20 EUR in LINK.

5. The minimum output amount is calculated as 240 EUR in PAXG.

6. The swap executes successfully (ignoring the incorrectly configured slippage parameters), but the vault only receives 236 EUR in PAXG.

Recommended Mitigation

Validate the slippage parameter minimumOutputAmount based on the delta vault token balance immediately before and after interaction with the swap router rather than relying solely on the behavior of the router itself.