Hajime - `rewards` is not calculated correctly

[sherlock-admin2]

Hajime

high

rewards is not calculated correctly

Summary

lastRewardPosition is not calculated correctly due to the zeroing of lastRewardGlobal

Vulnerability Detail

Here's an example: Bob calls the decreaseLiqudity() function, lastRewardGlobal reset:

 assetState_.lastRewardGlobal = 0;
 ...
 assetState[asset] = assetState_;

After Alice also called decreaseLiqudity(), in which lastRewardGlobal is already set to zero. This null value is passed to _getRewardBalances() :

 // Calculate the new reward balances.
 (assetState_, positionState_) = _getRewardBalances(assetState_, positionState_);

_getRewardBalances() calculates deltaReward. it will be equal to currentRewardGlobal. Because lastRewardGlobal = 0.

Delta is no longer the difference between different amounts:

  // Calculate the increase in rewards since last Asset interaction.
  uint256 deltaReward = currentRewardGlobal - assetState_.lastRewardGlobal;

Subsequently, deltaReward is used in the calculation deltaRewardPerToken:

 uint256 deltaRewardPerToken = deltaReward.mulDivDown(1e18, assetState_.totalStaked);

deltaRewardPerToken is used in the calculationlastRewardPerTokenGlobal :

unchecked {
                assetState_.lastRewardPerTokenGlobal =
                    assetState_.lastRewardPerTokenGlobal + SafeCastLib.safeCastTo128(deltaRewardPerToken);
            }

lastRewardPerTokenGlobal is used in the calculation deltaRewardPerToken

 unchecked {
                deltaRewardPerToken = assetState_.lastRewardPerTokenGlobal - positionState_.lastRewardPerTokenPosition;
            }

all these calculations lead to incorrect lastRewardPosition:

   unchecked {
                deltaReward = deltaRewardPerToken * positionState_.amountStaked / 1e18;
            }
      positionState_.lastRewardPosition =
                SafeCastLib.safeCastTo128(positionState_.lastRewardPosition + deltaReward);
        }

further in decreaseLiquidity() lastRewardPosition is equated to rewards:

rewards = positionState_.lastRewardPosition;

Impact

resetting lastRewardGlobal results in incorrect calculation of deltaReward and rewards

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/asset-modules/abstracts/AbstractStakingAM.sol#L396

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/asset-modules/abstracts/AbstractStakingAM.sol#L529-L569

Tool used

Manual Review

Recommendation

No need to reset lastRewardGlobal since its value is necessary for calculating the deltaReward. The value of which is key in calculating rewards. So the contract receives up-to-date information from LPStakingTime.sol about rewards.

Duplicate of #38

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(1)