User can withdraw more rewards than expected due to a miscalculation of the claimed array

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L63

Vulnerability details

Impact

In the _distributeETHRewardsToUserForToken() function, the claimed array is updated with the claimed rewards of each user. This array is also used to calculate how much awards a user can withdraw.

There is an error when the array is updated: instead of increasing the value with the withdrawn awards (due), the value is replaced with the withdrawn awards (due). The new value will then be lower than the expected one, allowing a user to withdraw more than due by withdrawing several times.

uint256 due = ((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token];
if (due > 0) {
    claimed[_user][_token] = due;   // should be "+= due"
    [...]
}

The issue can also prevent a user to interact with the contract and withdraw his own assets because _distributeETHRewardsToUserForToken() will revert if the calculated due is higher than the ETH balance in the contract.

Proof of Concept

Proof of concept using GiantMevAndFeesPool.claimRewards() but other paths are available.

• A user has a positive lpTokenETH balance on the GiantMevAndFeesPool contract, allowing him to withdraw awards
• The user call the claimRewards() function on the GiantMevAndFeesPool contract with the following parameters:
  • _recipient: user's address
  • _stakingFundsVaults: array with at least one available stacking fund. Note that the user can also provide an alternate contract which does not revert on claimRewards(address,bytes[]) as there is no verification here
  • _blsPublicKeysForKnots: related BLS keys (if using an available stacking fund)
• After the call, due rewards have been sent to the user and claimed[user][lpTokenEth] is now set to a lower value than expected
• When calling claimRewards() at a later time, due rewards will be miscalculated and be higher than expected
  • If there is enough ETH balance at that time, the related Ether will be sent to user
  • If there is not enough ETH balance, the transaction will revert. As _distributeETHRewardsToUserForToken() is called at other places in the contract, this will prevent the user to do some actions like withdrawing his own assets

Tools Used

Manual review.

Recommended Mitigation Steps

Fix the _distributeETHRewardsToUserForToken() function to increase claimed[_user][_token] with due instead of updating it with due.

[c4-judge]

dmvt marked the issue as duplicate of #59

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

JeeberC4 marked the issue as duplicate of #147