The Standard

Summary

A malicious staker can steal all of the native rewards (ETH for now), received by the LiquidationPool contract when liquidating an under-collateralized vault. This can achieved by utilizing the fact that there is no access control and/or input validation in LiquidationPool::distributeAssets. This in turn also denies all other stakers who are eligible for native rewards from receiving all of their asset rewards altogether.

Vulnerability Details

Stakers are users of the protocol which hold EUROs stablecoins and TST governance tokens. As a reward for staking those assets, by increasing their positions in LiquidationPool::increasePosition, they are eligible for fees (generated from borrowers minting, burning and swapping in their vaults), and the vaults' collateral assets when those are being liquidated due to the vault being under-collateralized. The vulnerability here is specifically in the distribution of liquidated assets among stakers.

When a vault is in an under-collateralized state, a user can call LiquidationPoolManager::runLiquidation, which will initiate a process that eventually will distribute the vault's collateral to the stakers, where each staker will receive his/her fair share of each token, where the fair share is the fraction of the total amount of a token multiplied by the ratio of the holder's stake to the total stake of all holders. This fair share, which in the code is denoted as _portion, is then set as the reward for that holder. When the holder will call LiquidationPool::claimRewards, they will receive those portions per each token. Also their EUROs position will be reduced by the portion (after converting it to its euro equivalent worth) multiplied by a discount rate, essentially implementing a mechanism where the holders buy the liquidated assets at a discount (with respect to their spot prices).

The vulnerability arises because LiquidationPool::distributeAssets is external with no access control and/or input validation therefore it allows a malicious staker to call this function with crafted input arguments to increase their portion of the native asset beyond their fair share and then call LiquidationPool::claimRewards to extract the total balance of native token held by the liquidation pool. Note that the portion of native token will increase for all holders but this is irrelevant at this point as they can do nothing with it because the pool has already been drained by the attacker.

This attack vector will only work for native tokens and not ERC20 tokens, because if the attacker will try to increase portions of ERC20 tokens as well, the pool will try to transfer those tokens from the manager and this will revert because the manager does not have those increased balances. The root of the problem is that the manager sends native token (ETH in this case) when it calls LiquidationPool::distributeAssets but the transfer of ERC20 tokens from the manager is implemented inside this function.

To see how this can go about, let’s look at the steps a malicious staker will take:

1. First step is to run the process of liquidating a vault that holds native tokens as part of its collateral by calling LiquidationPoolManager::runLiquidation. This step will send the ETH balance to the liquidation pool and distribute the assets as intended to the stakers.

2. The second step is for the staker to make sure they still hold EUROs and TST after the first step. If not then increase the position even by the slightest amount. This is only to make sure the malicious staker is eligible for rewards.

3. Third step is to call LiquidationPool::distributeAssets with the following arguments:

   1. An array of Asset structs with length one, meaning only the native token asset. This Asset struct is made of a Token struct and an amount.

   2. The Token struct variables should be crafted as follows:

      1. For bytes32 symbol → pass NATIVE - this is the symbol for ETH collateral in the protocol.

      2. For address addr → pass the zero address - this will prevent the code from trying to treat the token as an ERC20 one and transfer tokens from the manager as can be seen here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L229-L232

      3. For uint8 dec → pass 18 but it doesn’t really matter as we should shortly see.

      4. For address clAddr → pass the Chainlink ETH/USD price feed address just so that https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L218 will return some value. The exact value is not important as we shall shortly see.

      5. For address clDec → Doesn’t matter what to pass. Just make it 0.

   3. The Asset struct amount variable should be calculated, so that the portion that the attacker will eventually hold won't be more than the ETH balance of the pool, otherwise LiquidationPool::claimRewards will revert. We'll see this calculation in a bit.

   4. The _collateralRate argument can be set to anything as long as it's not zero.

   5. The _hundredPC argument should pass as zero - This will cause costInEuros to be equal to zero in https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L220C32-L220C32. This is why Token.dec, Token.clAddr and _collateralRate are not very important with what value we put in them.

Now , after calling LiquidationPool::distributeAssets with the crafted inputs, the _portion variable is under the attacker's control and therefore the reward they will get. This can be seen here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L219 and here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L227.

Note that the variable _nativePurchased will increase but it doesn't matter, because it is used in LiquidationPoolManager::returnUnpurchasedNative to return leftover of native token to the manager which happens when costInEuros > _position.EUROs as can be seen here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L222C21-L222C21. But in this case, costInEuros is zero and so _nativePurchased = asset.amount and the ETH transfer here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L199 will send 0 ETH to the manager, leaving the pool's ETH balance unchanged.

The last thing the attacker should take care of is passing the asset.amount argument that will result in his own portion not becoming larger than the pool’s ETH balance. This can be calculated via the equation uint256 _portion = asset.amount * _positionStake / stakeTotal;. The attacker needs beforehand to know what is the ETH balance of the pool (can be easily read from the blockchain), and their own position + already assigned ETH reward, which they can get from the external view function LiquidationPoolManager::position.

The only problem here is to determine the total stake which makes it a bit less comfortable to find exactly how much asset.amount they need to pass. To circumvent this, the attacker will pass small values of asset.amount and track their reward. Once it is close enough to the pool's balance they will stop and finally call LiquidationPool::claimRewards which will send them all of their rewards including the whole (or very close to it) ETH balance of the pool.

Except from the attacker stealing all native rewards, other stakers who are eligible for native rewards will not be able to claim all of their rewards because LiquidationPool::claimRewards will revert once it'll try to send a non-zero amount of ETH reward to the stakers, effectively DOSing the reward mechanism for stakers with native rewards assigned to them (note that stakers that do not have native token as reward will be able to claim their ERC20 asset rewards).

Impact

All native rewards (ETH) can be taken by a malicious staker, bypassing their fair share of the native asset. This will prevent other stakers from claiming all of their asset reward portions altogether, when part of those rewards is the native asset, therefore losing the rewards that they should have been eligible for.

Tools Used

Manual review

Recommendations

As far as I can tell, LiquidationPool::distributeAssets does not need to be called by regular users. Like the LiquidationPool::distributeFees, the LiquidationPool::distributeAssets should have the onlyManager modifier thus preventing users from calling it and preventing this attack vector altogether.