StakedStargateAM._getCurrentReward() fetches incorrect values, affecting rewards calculation.

Summary

StakedStargateAM._getCurrentReward() is currently implemented incorrectly, resulting in the retrieval of incorrect values. Consequently, AbstractStakingAM._getRewardBalances() will compute smaller values of rewards than anticipated.

Vulnerability Detail

Most of the functions in the AbstractStakingAM contract call internally _getRewardBalances() to update global and position-specific reward balances. To do so it first fetches the amounts of rewards in the staking contract and then subtracts from the rewards from the last update and proceds using this deltaReward value in the remaining calculations, as shown below.

    // Calculate the new assetState
    // Fetch the current reward balance from the staking contract.
    uint256 currentRewardGlobal = _getCurrentReward(positionState_.asset);
    // Calculate the increase in rewards since last Asset interaction.
    uint256 deltaReward = currentRewardGlobal - assetState_.lastRewardGlobal;
    uint256 deltaRewardPerToken = deltaReward.mulDivDown(1e18, assetState_.totalStaked);

However, the _getCurrentReward() method, as implemented by the Staked Stargate Asset Module, deviates from expectations by returning only the rewards accrued since the last claim.

    function _getCurrentReward(address asset) internal view override returns (uint256 currentReward) {
        currentReward = LP_STAKING_TIME.pendingEmissionToken(assetToPid[asset], address(this));
    }

    function pendingEmissionToken(uint256 _pid, address _user) external view returns (uint256) {
        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][_user];
        uint256 accEmissionPerShare = pool.accEmissionPerShare;
        uint256 lpSupply = pool.lpToken.balanceOf(address(this));
        if (block.timestamp > pool.lastRewardTime && lpSupply != 0 && totalAllocPoint > 0) {
            uint256 multiplier = getMultiplier(pool.lastRewardTime, block.timestamp);
            uint256 tokenReward = multiplier.mul(eTokenPerSecond).mul(pool.allocPoint).div(totalAllocPoint);
            accEmissionPerShare = accEmissionPerShare.add(tokenReward.mul(1e12).div(lpSupply));
        }
        return user.amount.mul(accEmissionPerShare).div(1e12).sub(user.rewardDebt);
    }

https://basescan.org/address/0x06Eb48763f117c7Be887296CDcdfad2E4092739C#code#F1#L122

Consider the code snippets above. StakedStargateAM._getCurrentReward() calls LP_STAKING_TIME.pendingEmissionToken(), which computes the total amounts of rewards subtracted by user.rewardDebt (the total amount previously claimed). Thus there is no need to calculate deltaReward value, as the value returned from _getCurrentReward() is the the increase in rewards since last asset interaction.

As a consequence _getRewardBalances() will calculate incorrectly deltaReward, leading to user receiving less rewards than expected. On some situations, when currentRewardGlobal < assetState_.lastRewardGlobal , which is possible if the interval between subsequent calls to _getRewardBalances() is small, the call will revert due to the undeflow in deltaReward calculation (deltaReward = currentRewardGlobal - assetState_.lastRewardGlobal;)

Impact

Stakers will receive less rewards than they are due and on some circumstances calls to _getRewardBalances() will revert.

Code Snippet

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

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/asset-modules/Stargate-Finance/StakedStargateAM.sol#L113-L115

Tool used

Manual Review

Recommendation

Considering that the StakedStargateAM contract is the only one who inherits from the AbstractStakingAM, the most straightforward fix is to obtain the deltaReward directly from _getCurrentReward().

diff --git a/AbstractStakingAM.sol b/AbstractStakingAM.mod.sol
index b9f29cc..f08fa64 100644
--- a/AbstractStakingAM.sol
+++ b/AbstractStakingAM.mod.sol
@@ -533,10 +533,8 @@ abstract contract StakingAM is DerivedAM, ERC721, ReentrancyGuard {
     {
         if (assetState_.totalStaked > 0) {
             // Calculate the new assetState
-            // Fetch the current reward balance from the staking contract.
-            uint256 currentRewardGlobal = _getCurrentReward(positionState_.asset);
             // Calculate the increase in rewards since last Asset interaction.
-            uint256 deltaReward = currentRewardGlobal - assetState_.lastRewardGlobal;
+            uint256 deltaReward = _getCurrentReward(positionState_.asset);
             uint256 deltaRewardPerToken = deltaReward.mulDivDown(1e18, assetState_.totalStaked);
             // Calculate and update the new RewardPerToken of the asset.
             // unchecked: RewardPerToken can overflow, what matters is the delta in RewardPerToken between two interactions.

Duplicate of #38

sherlock-audit / 2023-12-arcadia-judging

Tricko - StakedStargateAM._getCurrentReward() fetches incorrect values, affecting rewards calculation. #135