Rebasing tokens are not handled correctly

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-thorchain/blob/e3fd3c75ff994dce50d6eb66eb290d467bd494f5/chain/ethereum/contracts/THORChain_Router.sol#L54

Vulnerability details

Impact

Not handling rebasing tokens properly can result some vaults not being able to use their allowances due to lack of funds in the router or amount of rebasing tokens to get locked forever.

Proof of Concept

Rebasing tokens like stETH for example can change the users underlying balance in both directions due to adding rewards or slashing.

In such case it is not correct to store the allowances for vaults in a storage variable while using the raw version of stETH because the underlying balance of the Router contract will change over time due to Lido's protocol logic.

So if stETH contract balance increases over time even if all _vaultAllowance[vault][token] go to 0 (due to transfer out functions usage), there will be remaining stETH amounts that will get locked in the contract forever.

On the other hand if a slashing event occurs and all vaults try to reduce their _vaultAllowance[vault][token] to 0, not all will be able to do it. The last couple of vaults will not be able to do this because there won't be enough stETH left in the router.

Tools Used

Manual Review

Recommended Mitigation Steps

Rebasing tokens should be handled differently from normal ERC20 tokens. One solution I think of is when accepting such tokens they should be wrapped to their non rebasing version. This way _vaultAllowance logic will not break. In the case of stETH, the tokens should be converted to wstETH.

Assessed type

ERC20

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as not a duplicate

[c4-judge]

trust1995 marked the issue as duplicate of #85

[c4-judge]

trust1995 changed the severity to 3 (High Risk)