User can forfeit other user rewards

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/ExtraRewardsDistributor.sol#L127

Vulnerability details

Impact

User can forfeit other user rewards by giving a higher _startIndex in getReward function

Proof of Concept

1. Assume User B has not received any reward yet so that his userClaims[_token][User B]=0
2. User A calls getReward function with _account as User B and _startIndex as 5
3. This eventually calls _allClaimableRewards at ExtraRewardsDistributor.sol#L213 which computes epochIndex =5>0?5:0 = 5
4. Assuming tokenEpochs is 10 and latestEpoch is 8, so reward will computed from epoch 5 till epoch index 7 and _allClaimableRewards will return index as 7
5. _getReward will simply update userClaims[_token][User B] with 7
6. This is incorrect because as per contract User B has received reward from epoch 0-7 even though he only received reward for epoch 5-7

Recommended Mitigation Steps

Do not allow users to call getReward function for other users

[0xMaharishi]

This is a valid report, however, considering it is only related to the distribution of reward tokens, I have a hard time classifying this as high risk

[dmvt]

I agree with the high risk rating on this one. A third party could cause significant loss of expected reward funds for users across the entire protocol if so inclined.

[dmvt]

I view this to be eligible for the additional 20k AURA bonus

This is in scope specifically because it is state in the file comments:

Allows anyone to distribute rewards to the AuraLocker at a given epoch.

Accordingly, I'm viewing these funds as part of the AuraLocker. This exploit allows them to be frozen.

[0xMaharishi]

This is absolutely not eligible for the additional bonus.

It does not affect user deposits.

These funds are not part of the AuraLocker at all. It simply allows users who have a balance in the AuraLocker to claim said rewards.

[dmvt]

It does not affect user deposits.

My mistake, you are correct. This is out of scope. I was reading it as frozen user funds, which is not what you wrote.

TBC, This is NOT eligible for the bonus.

[sparrowDom]

Hey, are there any plans to address this issue maybe? Possible solutions:

• make it possible only for the owner of the account to claim the rewards
• if above isn't possible for various reasons, make it default that either:
  • only owner can claim the tokens (and the owner can opt out of this option)
  • anyone can claim tokens for owner (and the owner can opt in so only owner is able to claim tokens)
• only owner of the account can override the _startIndex of the rewards harvest. Non owners execute the call using less gas efficient start epoch: epochIndex = userClaims[_token][_account

[0xMaharishi]

@sparrowDom You can see solution here https://github.com/aurafinance/aura-contracts/blob/main/contracts/rewards/ExtraRewardsDistributor.sol#L141 makes it such that only the owner can use a custom start index

[sparrowDom]

Yes you are right thanks!