StakingRewardsV2 does not impose any restriction regarding esLBRBoost unlock time

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol#L101-L108
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol#L56-L66
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol#L110-L118
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol#L92-L99
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L87-L98

Vulnerability details

Impact

Users can claim rewards and withdraw their stake on StakingRewardsV2 without any restriction, while having the incentives from the boost of esLBRBoost.

Proof of Concept

Steps to reproduce:

• Set the lock status to its max value via esLBRBoost::setLockStatus()
• Stake via StakingRewardsV2::stake()
• Wait some time
• Claim rewards via StakingRewardsV2::getReward() -> This will be calculated with the boost
• Withdraw via StakingRewardsV2::withdraw()

The result will be that the user will get the benefit from the boost while claiming rewards, despite not commiting to its unlock time, as there is no check on StakingRewardsV2 to prevent it.

esLBRBoost provides a boost to rewards on getBoost(), which is used by earned(), and the modifier updateReward():

    function getBoost(address _account) public view returns (uint256) {
        return 100 * 1e18 + esLBRBoost.getUserBoost(_account, userUpdatedAt[_account], finishAt);
    }

    // Calculates and returns the earned rewards for a user
    function earned(address _account) public view returns (uint256) {
        return ((balanceOf[_account] * getBoost(_account) * (rewardPerToken() - userRewardPerTokenPaid[_account])) / 1e38) + rewards[_account];
    }

    modifier updateReward(address _account) {
        rewardPerTokenStored = rewardPerToken();
        updatedAt = lastTimeRewardApplicable();

        if (_account != address(0)) {
@>          rewards[_account] = earned(_account); // @audit using earned() => uses the boost underneath
            userRewardPerTokenPaid[_account] = rewardPerTokenStored;
            userUpdatedAt[_account] = block.timestamp;
        }
        _;
    }

stakerewardV2pool.sol#L101-L108

stakerewardV2pool.sol#L56-L66

getReward() doesn't have any check to prevent the user from claiming rewards regarding the boost lock. withdraw() doesn't have any related check either:

    // Allows users to claim their earned rewards
    function getReward() external updateReward(msg.sender) {
        uint256 reward = rewards[msg.sender];
        if (reward > 0) {
            rewards[msg.sender] = 0;
            rewardsToken.mint(msg.sender, reward);
            emit ClaimReward(msg.sender, reward, block.timestamp);
        }
    }

    // Allows users to withdraw a specified amount of staked tokens
    function withdraw(uint256 _amount) external updateReward(msg.sender) {
        require(_amount > 0, "amount = 0");
        balanceOf[msg.sender] -= _amount;
        totalSupply -= _amount;
        stakingToken.transfer(msg.sender, _amount);
        emit WithdrawToken(msg.sender, _amount, block.timestamp);
    }

stakerewardV2pool.sol#L110-L118

stakerewardV2pool.sol#L92-L99

For reference, this is how the current deployed version of StakingRewardsV2 handles this for Lybra v1. It checks the unlock time via a requirement block.timestamp >= esLBRBoost.getUnlockTime(msg.sender):

    // Allows users to claim their earned rewards
    function getReward() external updateReward(msg.sender) {
@>      require(
@>          block.timestamp >= esLBRBoost.getUnlockTime(msg.sender),              // @audit
@>          "Your lock-in period has not ended. You can't claim your esLBR now."
@>      );
        uint256 reward = rewards[msg.sender];
        if (reward > 0) {
            rewards[msg.sender] = 0;
            lybraFund.refreshReward(msg.sender);
            rewardsToken.mint(msg.sender, reward);
        }
    }

https://etherscan.io/address/0xBAAA3053e773544561a119DB1F86985581D3fE7F?utm_source=immunefi#code#F1#L182

For reference, ProtocolRewardsPool on the current audit, handles it on the unstake() function:

    function unstake(uint256 amount) external {
@>       require(block.timestamp >= esLBRBoost.getUnlockTime(msg.sender), "Your lock-in period has not ended. You can't convert your esLBR now."); // @audit
        esLBR.burn(msg.sender, amount);
        withdraw(msg.sender);
        uint256 total = amount;
        if (time2fullRedemption[msg.sender] > block.timestamp) {
            total += unstakeRatio[msg.sender] * (time2fullRedemption[msg.sender] - block.timestamp);
        }
        unstakeRatio[msg.sender] = total / exitCycle;
        time2fullRedemption[msg.sender] = block.timestamp + exitCycle;
        emit UnstakeLBR(msg.sender, amount, block.timestamp);
    }

ProtocolRewardsPool.sol#L87-L98

Tools Used

Manual review

Recommended Mitigation Steps

Depending on protocol decision, check the esLBRBoost unlock time in StakingRewardsV2 when trying to get rewards, or prevent them from withdrawing, forcing them to comply with their commitment.

Assessed type

Invalid Validation

[c4-pre-sort]

JeffCX marked the issue as primary issue

[c4-sponsor]

LybraFinance marked the issue as sponsor disputed

[LybraFinance]

That's how we designed it.

[0xean]

@LybraFinance - your response seems different on this issue #773
which I think is a dupe, can you please clarify?

[LybraFinance]

Because the boost locking in this version is only restricted to the conversion from esLBR to LBR and does not affect the rewards claiming in StakingRewardsV2. So, this is not an unexpected issue.

[0xean]

@LybraFinance -

I am still not following why this issue (#838) is different than #773 which you confirmed. From my understanding, both of the issues highlight the ability of the user to withdraw their staking token regardless of the lock.

[c4-sponsor]

LybraFinance marked the issue as sponsor confirmed

[c4-judge]

0xean marked the issue as duplicate of #773

[c4-judge]

0xean marked the issue as satisfactory