code-423n4 / 2022-05-aura-findings

0 stars 1 forks source link

Re-entrancy on `BaseRewardPool.getReward()` #360

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/BaseRewardPool.sol#L289

Vulnerability details

See @audit-info tags:

File: BaseRewardPool.sol
280:     /**
281:      * @dev Gives a staker their rewards, with the option of claiming extra rewards
282:      * @param _account     Account for which to claim
283:      * @param _claimExtras Get the child rewards too?
284:      */
285:     function getReward(address _account, bool _claimExtras) public updateReward(_account) returns(bool){
286:         uint256 reward = earned(_account);
287:         if (reward > 0) {
288:             rewards[_account] = 0;
289:             rewardToken.safeTransfer(_account, reward);  // @audit-info checks-effects-interractions not respected, consider adding a reentrancy guard
290:             IDeposit(operator).rewardClaimed(pid, _account, reward);
291:             emit RewardPaid(_account, reward);
292:         }
293: 
294:         //also get rewards from linked rewards
295:         if(_claimExtras){
296:             for(uint i=0; i < extraRewards.length; i++){
297:                 IRewards(extraRewards[i]).getReward(_account);
298:             }
299:         }
300:         return true;
301:     }

Mitigations

Consider moving transfer of tokens at the final and add a reentrancy guard.

dmvt commented 2 years ago

Once again the contract in question is in complete control of the protocol. Invalid report.