The Standard

Summary

Since PAXG is a fee-on-transfer token, the LiquidationPool will never hold the needed amount of PAXG to pay all of the stakers eligible for rewards, causing some stakers, essentially the last ones who wish to claim their rewards, not be able to claim all of their rewards because once the contract will not have enough PAXG in its balance, the LiquidationPool::claimRewards function will revert.

Vulnerability Details

When borrowers create their vaults, they need to transfer collateral to the vault in order to be allowed to mint EUROs stablecoins. Currently, the protocol receives a few tokens, one of them is PAXG which is an ERC-20 token but with the fee-on-transfer feature. This means that every time a call to the transfer function in PAXG contract is made, the amount to transfer is not the amount that the receiving address will receive because a fraction of this amount, a fee, is deducted and sent to PAXOS company.

This mechanism needs to be dealt with when interacting with PAXG contract, specifically making sure that the handled amount is the actual amount transferred, meaning the amount minus the fee.

The Standard protocol handles transfers of the collateral tokens, including PAXG, when a vault is liquidated due to it being under-collateralized. Specifically this is done in LiquidationPool::distributeAssets as can be seen in https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L232. In this line of code, the liquidation pool will transfer the tokens from the manager to itself with the amount being _portion. _portion represents the fair share of a holder, depending on his stake in relation to the total stake of all holders. This _portion is the reward that the holder is eligible for and this is indeed the case as can be seen here https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L227.

The problem arises because there is a discrepancy between the PAXG rewards of the holders and the actual amount the pool holds. The pool does not hold _portion of PAXG (per holder), but a lower amount because of the fee deducted. So the total amount of PAXG in the pool is not sufficient to pay the rewards to all the holders. By the time the last holder(s) will try to claim their reward through LiquidationPool::claimRewards, the pool will not have enough balance to cover the transfer and the function will revert, which will deny those holders from receiving any reward whatsoever.

A POC might look like this:

1. Create a vault by calling SmartVaultManagerV5::mint.

2. Transfer PAXG to the vault as collateral (and you might transfer other tokens as well to prove the point that all rewards will be lost).

3. Under-collateralize the vault.

4. Create a number of addresses representing stakers with some TST and EUROs staked for each (and wait 24 hours due to pending restriction).

5. Liquidate the vault by calling LiquidationPoolManager::runLiquidation.

6. Call LiquidationPool::claimRewards multiple times, depends on how much holders you defined.

7. The LiquidationPool::claimRewards function will revert for (at least) the last holder. This holder(s) cannot claim their fair share of rewards.

Impact

Once most stakers claim their rewards, and the PAXG balance of the LiquidationPool contract dwindled, the last stakers eligible for rewards will lose all of them because the contract won't have enough PAXG balance and therefore LiquidationPool::claimRewards function will revert.

Tools Used

Manual review

Recommendations

Add an if .. else… statement in LiquidationPool::distributeAssets function to check if the current asset is PAXG and if it is then update the rewards mapping with _portion * (1 - fee), where the fee should be read from the PAXG contract.

Another option is to add code to actually check the balance_before and balance_after when the token distributed is PAXG, and update the rewards mapping accordingly.