0xGoodess - the stake flow on gpToke being mandatory on rewarder potentially brick staker's withdrawal from DestinationVault

[sherlock-admin2]

0xGoodess

medium

the stake flow on gpToke being mandatory on rewarder potentially brick staker's withdrawal from DestinationVault

Summary

the stake flow on gpToke being mandatory on AbstractRewarder potentially brick staker's withdrawal

Vulnerability Detail

Let's look at the withdrawal flow (withdrawUnderlying/withdrawBaseAsset)

1. invoke _burn(msg.sender, shares)
2. invoke _beforeTokenTransfer, since _burn would transfer the token from the user to address(0)
3. invoke _rewarder.withdraw(from, amount, true);, where claim(the last input) is set to true
4. Over to MainRewarder, withdraw would trigger _processRewards if claim is set to true, which is indeed the case.
5. _processRewards would call _getReward
6. Over to AbstractRewarders, _getReward would check if the rewardToken is TOKE, if so and a non-zero lcokDuration is set, the workflow would attempt to help the user to lock the TOKE reward, calling the function gptoke.stake
7. Over to GPToke, stake would call _stake which has 3 constraints:

• A: amount cannot be smaller than 10_000
• B: amount cannot be larger than 100m * 1e18
• C: the GPToke cannot be paused.

GPToken::_stake

    function _stake(uint256 amount, uint256 duration, address to) internal whenNotPaused {
        //
        // validation checks
        //
        if (to == address(0)) revert ZeroAddress();
        if (amount < MIN_STAKE_AMOUNT) revert StakingAmountInsufficient();
        if (amount > MAX_STAKE_AMOUNT) revert StakingAmountExceeded();

        // duration checked inside previewPoints
        (uint256 points, uint256 end) = previewPoints(amount, duration);

        if (points + totalSupply() > type(uint192).max) {
            revert StakingPointsExceeded();
        }

        // checkpoint rewards for caller
        _collectRewards(to, false);

        // save information for current lockup
        lockups[to].push(Lockup({ amount: uint128(amount), end: uint128(end), points: points }));

        // create points for user
        _mint(to, points);

        emit Stake(to, lockups[to].length - 1, amount, end, points);

        // transfer staked toke in
        toke.safeTransferFrom(msg.sender, address(this), amount);
    }

MainRewarder

    function withdraw(address account, uint256 amount, bool claim) public onlyStakeTracker {
        _updateReward(account);
        _withdraw(account, amount);

        for (uint256 i = 0; i < extraRewards.length; ++i) {
            IExtraRewarder(extraRewards[i]).withdraw(account, amount);
        }

        if (claim) {
            _processRewards(account, true);
        }
    }

DestinationVault

    function withdrawUnderlying(uint256 shares, address to) external onlyLMPVault returns (uint256 amount) {
        Errors.verifyNotZero(shares, "shares");
        Errors.verifyNotZero(to, "to");

        amount = shares;

        emit UnderlyingWithdraw(amount, msg.sender, to);

        // Does a balance check, will revert if trying to burn too much
        _burn(msg.sender, shares);

        _ensureLocalUnderlyingBalance(amount);

        IERC20(_underlying).safeTransfer(to, amount);
    }

...

    function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override {
        if (from == to) {
            return;
        }

        if (from != address(0)) {
            _rewarder.withdraw(from, amount, true);
        }
    }

Impact

stakers who participate in LMPVault which contributes to DestinationVault where rewardToken is Toke can be bricked from withdrawal when GPToke is paused from accepting new stakes, or not enough TOKE is sent in timely as reward to reach the minimum threshold for the staker.

While the minimum requirement(10_000) is fairly negligible, it may still affect stakers who :
1.) staker who just stake for a very short period of time (1 block).
2.) staker who just have a very small amount of deposit.
3.) pausing GPToken would pause withdrawal in DestinationVault, thus GMTVault too.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/DestinationVault.sol#L341

Tool used

Manual Review

Recommendation

There are 2 possible ways to work around this:

1. change the claim input in _beforeTransfer to be false, so that user need to claim their only claimable after withdrawal.
2. make exception on the GPToke::_stake function such that the above constraint does not apply to any TOKE Rewarder.

Duplicate of #565