sherlock-audit / 2024-05-elfi-protocol-judging

11 stars 7 forks source link

aman - User will loss the Rewards for stacking if he redeem without claiming the reward #252

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

aman

Medium

User will loss the Rewards for stacking if he redeem without claiming the reward

Summary

Rewards for staked tokens accumulate over time based on the duration of the staking period. However, Due to an Issue in code The Rewards can not be claimed more details in next section.

Vulnerability Detail

The Protocol Provide Rewards token as incentive for users to stake at thre platform. To stake token user will call createMintStakeTokenRequest and ROLE_KEEPER will call executeMintStakeToken Here we also update the rewards for staking via updateAccountFeeRewards.

function updateAccountFeeRewards(address account, address stakeToken) public {
        StakingAccount.Props storage stakingAccount = StakingAccount.load(account);
        StakingAccount.FeeRewards storage accountFeeRewards = stakingAccount.getFeeRewards(stakeToken);
        FeeRewards.MarketRewards storage feeProps = FeeRewards.loadPoolRewards(stakeToken);
        if (accountFeeRewards.openRewardsPerStakeToken == feeProps.getCumulativeRewardsPerStakeToken()) {
            return;
        }
        uint256 stakeTokens = IERC20(stakeToken).balanceOf(account);
        if (
            stakeTokens > 0 &&
            feeProps.getCumulativeRewardsPerStakeToken() - accountFeeRewards.openRewardsPerStakeToken >
            feeProps.getPoolRewardsPerStakeTokenDeltaLimit()
        ) {
            accountFeeRewards.realisedRewardsTokenAmount += (
                stakeToken == CommonData.getStakeUsdToken()
                    ? CalUtils.mul(
                        feeProps.getCumulativeRewardsPerStakeToken() - accountFeeRewards.openRewardsPerStakeToken,
                        stakeTokens
                    )
                    : CalUtils.mulSmallRate(
                        feeProps.getCumulativeRewardsPerStakeToken() - accountFeeRewards.openRewardsPerStakeToken,
                        stakeTokens
                    )
            );
        }
        accountFeeRewards.openRewardsPerStakeToken = feeProps.getCumulativeRewardsPerStakeToken();
        stakingAccount.emitFeeRewardsUpdateEvent(stakeToken);
    }

It can be observed from above function that the realisedRewardsTokenAmount will only be update if stakeTokens>0. So in our case it will not be updated. after some time user creates a redeem request to redeem stack tokens. No in case of Redeem we update the rewards after redeeming.

function executeRedeemStakeToken(uint256 requestId, Redeem.Request memory redeemRequest) external {
        uint256 redeemAmount;
        if (CommonData.getStakeUsdToken() == redeemRequest.stakeToken) {
            redeemAmount = _redeemStakeUsd(redeemRequest);
        } else if (CommonData.isStakeTokenSupport(redeemRequest.stakeToken)) {
            redeemAmount = _redeemStakeToken(redeemRequest);
        } else {
            revert Errors.StakeTokenInvalid(redeemRequest.stakeToken);
        }

@>      FeeRewardsProcess.updateAccountFeeRewards(redeemRequest.account, redeemRequest.stakeToken);

        Redeem.remove(requestId);

        emit RedeemSuccessEvent(requestId, redeemAmount, redeemRequest);
    }

Here We can see that When we redeem the Tokens the User stack tokens can be zero , So the realisedRewardsTokenAmount can not be updated in this case. Now lets check what happens if user Tries to Claim Rewards Tokens he Will call ClaimRewards

function claimRewards(uint256 requestId, ClaimRewards.Request memory request) external {
        address account = request.account;
        address[] memory stakeTokens = CommonData.getAllStakeTokens();
        StakingAccount.Props storage stakingAccount = StakingAccount.load(account);
        StakingAccount.FeeRewards storage accountFeeRewards;
        address[] memory allStakeTokens = new address[](stakeTokens.length + 1);
        uint256[] memory claimStakeTokenRewards = new uint256[](stakeTokens.length + 1);
        for (uint256 i; i < stakeTokens.length; i++) {
            address stakeToken = stakeTokens[i];
            allStakeTokens[i] = stakeToken;
            FeeRewards.MarketRewards storage feeProps = FeeRewards.loadPoolRewards(stakeToken);
            FeeRewardsProcess.updateAccountFeeRewards(account, stakeToken);
            accountFeeRewards = stakingAccount.getFeeRewards(stakeToken);
@>            if (accountFeeRewards.realisedRewardsTokenAmount == 0) {
                continue;
            }
        ....
    }

In above code when realisedRewardsTokenAmount==0 we can not Process it.

  1. Bob Stack 10e18 for 10 days.
  2. Bob initial Stake=0 so his realisedRewardsTokenAmount will not be updated.
  3. After 10 days Bob redeem stack tokens.
  4. we first redeem Bob token so stake=0.
  5. The realisedRewardsTokenAmount will not be updated and remain 0.

Impact

The User will loos the rewards token for staking.

Code Snippet

https://github.com/sherlock-audit/2024-05-elfi-protocol/blob/main/elfi-perp-contracts/contracts/process/RedeemProcess.sol#L78

Tool used

Manual Review

Recommendation

First updateAccountFeeRewards then redeem tokens.

diff --git a/elfi-perp-contracts/contracts/process/RedeemProcess.sol b/elfi-perp-contracts/contracts/process/RedeemProcess.sol
index dedfe16e..8c293d07 100644
--- a/elfi-perp-contracts/contracts/process/RedeemProcess.sol
+++ b/elfi-perp-contracts/contracts/process/RedeemProcess.sol
@@ -67,6 +67,8 @@ library RedeemProcess {

     function executeRedeemStakeToken(uint256 requestId, Redeem.Request memory redeemRequest) external {
         uint256 redeemAmount;
+        FeeRewardsProcess.updateAccountFeeRewards(redeemRequest.account, redeemRequest.stakeToken);
+
         if (CommonData.getStakeUsdToken() == redeemRequest.stakeToken) {
             redeemAmount = _redeemStakeUsd(redeemRequest);
         } else if (CommonData.isStakeTokenSupport(redeemRequest.stakeToken)) {
@@ -75,7 +77,6 @@ library RedeemProcess {
             revert Errors.StakeTokenInvalid(redeemRequest.stakeToken);
         }

-        FeeRewardsProcess.updateAccountFeeRewards(redeemRequest.account, redeemRequest.stakeToken);

Duplicate of #274