howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e5ae503d0dc2394a82242be6860eb538345152a1/ethereum/contracts/THORChain_Router.sol#L156-L157

Vulnerability details

Some tokens may make arbitrary balance modifications outside of transfers (e.g. Ampleforth style rebasing tokens, Compound style airdrops of governance tokens, mintable/burnable tokens).

The THORChain Router contract uses the _vaultAllowance mapping to track token balances (i.e. vault allowances). Thus, arbitrary modifications to underlying token balances can mean that the contract is operating with outdated information.

If the actual token balance in the Router is less than the value tracked in _vaultAllowance, then transferAllowance, transferOut, transferOutV5, transferOutAndCall, transferOutAndCallV5 could fail when the vault has enough allowance but the underlying token balance in the Router is insufficient for the transaction.

Impact

ERC20 tokens that modify balances outside of transfers lead to broken functionality in the Router contract.

Proof of Concept

The Ampleforth token is such a rebase token in the whitelist provided on the contest page.

After the user deposit the Ampleforth token, the transferred amount is recorded in _vaultAllowance. The Ampleforth token could then rebase negatively (see the rebase history, resulting in the underlying Ampleforth balance in the Router to be less than the amount stored in _vaultAllowance. Consequently, some vault allowance in the Router is unbacked by the underlying token, preventing the vaults from using their full allowance to transfer funds out.

Tools Used

Manual Review

Recommended Mitigation Steps

Exclude ERC20 tokens that modify balances outside of transfers from the token whitelist.