Description
Even when a user calls withdraw(deCRVUSDAmount) with a paramater deCRVUSDAmount = 1, their entire RAAC reward is transferred to them. This is because calculateRaacRewards() calculates the rewards on the user's entire deposit.
Hence attacker can make the call multiple times and drain all the rewards, effectively stealing the reward of others.
File: contracts/core/pools/StabilityPool/StabilityPool.sol
224: function withdraw(uint256 deCRVUSDAmount) external nonReentrant whenNotPaused validAmount(deCRVUSDAmount) {
225: _update();
226: if (deToken.balanceOf(msg.sender) < deCRVUSDAmount) revert InsufficientBalance();
227:
228: uint256 rcrvUSDAmount = calculateRcrvUSDAmount(deCRVUSDAmount);
229:@---> uint256 raacRewards = calculateRaacRewards(msg.sender);
230: if (userDeposits[msg.sender] < rcrvUSDAmount) revert InsufficientBalance();
231: userDeposits[msg.sender] -= rcrvUSDAmount;
232:
233: if (userDeposits[msg.sender] == 0) {
234: delete userDeposits[msg.sender];
235: }
236:
237: deToken.burn(msg.sender, deCRVUSDAmount);
238: rToken.safeTransfer(msg.sender, rcrvUSDAmount);
239: if (raacRewards > 0) {
240:@---> raacToken.safeTransfer(msg.sender, raacRewards);
241: }
242:
243: emit Withdraw(msg.sender, rcrvUSDAmount, deCRVUSDAmount, raacRewards);
244: }
245:
246:
247: * @notice Calculates the pending RAAC rewards for a user.
248: * @param user Address of the user.
249: * @return Amount of RAAC rewards.
250: */
251: function calculateRaacRewards(address user) public view returns (uint256) {
252:@---> uint256 userDeposit = userDeposits[user];
253: uint256 totalDeposits = deToken.totalSupply();
254:
255: uint256 totalRewards = raacToken.balanceOf(address(this));
256: if (totalDeposits < 1e6) return 0;
257:
258:@---> return (totalRewards * userDeposit) / totalDeposits;
259: }
Attack Path:
Alice the honest user has deposited 100 rTokens and received 100 deCRVUSDAmount
Bob the attacker has deposited 100 rTokens too and received 100 deCRVUSDAmount
The raacToken.balanceOf(address(this)) is currently 300
Bob calls withdraw(1) and receives 1 rToken back. However he also receives his entire rewards of (300 * 100) / 200 = 150
Bob again calls withdraw(1) and receives 1 rToken back. He again receives rewards of (150 * 99) / 199 = ~ 75
Bob repeats the process again and again until raacToken.balanceOf(address(this)) < 1e6 and calculateRaacRewards() returns zero
Alice has no reward left to claim now, while Bob has profited at her expense.
Impact
Entire RAAC rewards can be drained.
Mitigation
File: contracts/core/pools/StabilityPool/StabilityPool.sol
224: function withdraw(uint256 deCRVUSDAmount) external nonReentrant whenNotPaused validAmount(deCRVUSDAmount) {
225: _update();
226: if (deToken.balanceOf(msg.sender) < deCRVUSDAmount) revert InsufficientBalance();
227:
228: uint256 rcrvUSDAmount = calculateRcrvUSDAmount(deCRVUSDAmount);
- 229: uint256 raacRewards = calculateRaacRewards(msg.sender);
+ 229: uint256 raacRewards = calculateRaacRewards(rcrvUSDAmount);
230: if (userDeposits[msg.sender] < rcrvUSDAmount) revert InsufficientBalance();
231: userDeposits[msg.sender] -= rcrvUSDAmount;
232:
233: if (userDeposits[msg.sender] == 0) {
234: delete userDeposits[msg.sender];
235: }
236:
237: deToken.burn(msg.sender, deCRVUSDAmount);
238: rToken.safeTransfer(msg.sender, rcrvUSDAmount);
239: if (raacRewards > 0) {
240: raacToken.safeTransfer(msg.sender, raacRewards);
241: }
242:
243: emit Withdraw(msg.sender, rcrvUSDAmount, deCRVUSDAmount, raacRewards);
244: }
245:
246: /**
247: * @notice Calculates the pending RAAC rewards for a user.
248: * @param user Address of the user.
249: * @return Amount of RAAC rewards.
250: */
- 251: function calculateRaacRewards(address user) public view returns (uint256) {
- 252: uint256 userDeposit = userDeposits[user];
+ 251: function calculateRaacRewards(uint256 forAmount) public view returns (uint256) {
+ 252: uint256 userDeposit = forAmount;
253: uint256 totalDeposits = deToken.totalSupply();
254:
255: uint256 totalRewards = raacToken.balanceOf(address(this));
256: if (totalDeposits < 1e6) return 0;
257:
258: return (totalRewards * userDeposit) / totalDeposits;
259: }
Also, we should change this:
File: contracts/core/pools/StabilityPool/StabilityPool.sol
261: /**
262: * @notice Gets the pending RAAC rewards for a user.
263: * @param user Address of the user.
264: * @return Amount of pending RAAC rewards.
265: */
266: function getPendingRewards(address user) external view returns (uint256) {
- 267: return calculateRaacRewards(user);
+ 267: return calculateRaacRewards(userDeposits[user]);
268: }
