Core Contracts

Summary

There is a critical vulnerability that lies in the logic of calculateRaacRewards function of the StabilityPool.sol that a malicious user can exploit to basically make the veRAACToken.sol useless or inoperable.specifically the lock function.

Vulnerability Details

The StabilityPool.sol is used by the users to deposit their rTokens and withdraw those tokens to get minted with raacRewards and with those rewards the users can take them and use them in the veRAACToken contract use the lock function to lock those tokens in order to get votingPower that can be used in the governance or other activities of the protocol. I will explain the vulnerability with an example step by step:

1. An attacker comes and deposits rTokens in the StabilityPool and then some time later calls the withdraw function to withdraw those tokens and get raacRewards along with them.

2. Now the issue is in the calculaeRaacRewards function that is used to compute the no. of rewards that the user should get at the time of withdrawing. The formoula is- (totalRewards * userDeposit) / totalDeposits; And this line in the withdraw function is also the reason for this vuln to be exploited by an attacker as can be seen in the above formula it takes the user's total deposits in account everytime while computing the rewards. An attacker can exploit this with withdrawing so much 1 wei of their deposited rTokens and everytime they will get rewards according to their totalDeposits. I will explain step by step what will happen.

3. Attacker deposited 100000e18 rTokens and got minted the same amount of deTokens as the exchange rate for them is 1:1 with rTokens and the raacRewards are also in the contract as everytime one calls deposits this function is called i.e for minting the rewards in this contract.

4. Now the attcker decides to withdraw his tokens but what he will do is instead of withdrawing the whole amount he will withdraw lets say for simplicity dollar by dollar like 1 rToken at a time and he does it for eg- 10k times, now as in the reward calculation the totalDeposits of user is taken the calculation will be like: for eg- lets say the:
   totalRewards = 1000000, userDeposits = 100000, totalDeposits = 1000000
(1000000 * 100000) / 1000000 = 100000 This is even if the user only withdraws 1 rToken. Now due to the above logic lets say the user does this 10000 times and everytime he gets the same amount of rewards i.e 100000 now 100000 * 10000 = 1000000000 as can be clearly seen this amount is 1 billion rewards if the user keeps withdrawing. Now lets keep in mind these 1 billion rewards and take a look at the veRAACToken.sol lock function which will be exploited in this case. Now what the attacker will do is go to the veRAACToken contract and call the lock function their make approx 9-10 calls to the function one by one and everytime he will deposit 10 million of those rewards as the max amount the user can lock at once is 10 m the attcker will keep calling the function till the totalSupply reaches till 100 million veRAACTokens, now after this the lock function will be inoperable due to this line meaning that now anytime a user comes to lock their rewards, the transaction will be reverted as the totalSupply will be at max i.e 100_000_000e18 therefore the attacker will end up DOS'ing the veRAAC contract. Another reason for this is also that even though the voting power reduces linearly with time the totalSupply will not go down until the withdraw function is called in the veRAACToken.sol so the attacker will never call that function and end up messing the protocol

Impact

Impact is critical here as the contract will be basically become useless and inoperable by other users

Tools Used

Manual Review

Recommendations

The intentions of the protocol are not clear yet, so any fix for this will be difficult without introducing other vulnerability but either of these things can be done: 1. The first and foremost that is suggested is that instead of making the revert at that MAX_TOTAL_SUPPLY line enforce something on the votingPower because that reduces linearly over time or what can be done is add some code or implementation like as the votingPower decreases, the totalSupply should also go down along with that, in actuality the main thing here is the votingPower not the totalSupply that will matter.
Secondly, Only allow single claims, i.e after one withdraw in the StablityPool do not allow claims of rewards after that so that the user will be forced to withdraw all of his tokens. Although the best option might be to fix or have some changes in the above MAX_TOTAL_SUPPLY that is causing the revert statement providing the fact that the totalSupply doesnt really matter here, what matters is the votingPower which is reduced as the time passes. instead of this line, enforce something on the votingPower like if after the lock votingPower gets to a certain level, then revert. The reason for this is because that MAX_TOTAL_SUPPLY line seems to benefit the wealthy users more as they can deposit or lock tokens in surplus and getting the power over governance too irrespective of the fact they are malicious or not

Code Snippets