https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L205-L241
By calling the distributeAssets() function with specific parameters, a malicious actor can increase its rewards as much as he wants before claiming them using the claimRewards() function.
As a result, the total amount of assets accrued from the fees and the vault liquidations can be stolen by the attacker.
Assume Eve and 9 other users have staked the following in the LiquidationPool contract :
100 TST
100 EUROs
Eve owns 10% of the stakeTotal.
After 1 day, her position is now eligible to receive rewards as her pendingStakes have passed the required deadline specified in the consolidatePendingStakes() function.
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L119-L132
Assume the LiquidationPool contract has accumulated the following through vaults liquidation :
10 WBTC (this can be any ERC20 token allowed by the protocol)
Eve wants to increase her rewards for the WBTC token and crafts a specific set of parameters for the distributeAssets() function before executing it :
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L205-L241
The distributeAssets() function will first calculate the stakeTotal variable using the getStakeTotal() function which will equal 1000.
After that, the function will iterate through every single holder (meaning they will also benefit from the vulnerability in some way) and set the _positionStake to 100 since every of the 10 holders (including Eve) holds the same amount of tokens.
Then, it will iterate (only once) through the malicious _asset array, cache the asset variable and set the _portion as follows
Now, the costInEuros is calculated and is meant to be subtracted from the holder's position BUT because Eve supplied an enormous uint256 as _collateralRate, the calculation will return 0. This means, nothing will be subtracted from the holder's position. Moreover, this costInEuros is supposed to be used to burn some EUROs token right after, which won't happen.
Here is a PoC in chisel that will calculate the costInEuros variable with the malicious parameters using fictive (but close to reality) WBTC/USD and USD/EUR prices
Finally, the rewards will be increased by the _portion (1 WBTC) and the amount of WBTC will be transferred from the LiquidationPoolManager to the LiquidationPool.
Eve can now execute the exact same transaction 9 more times (for a total of 10 times) to increase her rewards up to 10 WBTC.
Since her rewards for WBTC have now reached 10 WBTC, if she calls the claimRewards() function, 10 WBTC will be transferred to her, leaving nothing for the other holders.
Note that any of the other holders could have called claimRewards() as well in order to grab the 10 WBTC
Manual analysis
Foundry (chisel)
Depending on the protocol, here are 2 possibilities :
Add access control to the distributeAssets() function to allow only trusted parties to execute it (e.g. the DAO).
Calculate the _collateralRate and _hundredPC internally instead of providing them as function parameters
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.