GPToke Rewards Collection Can Lead to Massive Loss of User Funds

Summary

The GPToke staking contract contains a critical bug in the _collectRewards() internal function that could lead to irrecoverable loss of user funds. If the reward transfer fails for any reason, a user's unclaimedRewards balance is reset before the transfer occurs, resulting in potentially unlimited rewards being lost. This poses a major risk to user funds and trust in the platform.

Vulnerability Detail

The _collectRewards() function first calculates any pending rewards due to the user, which includes:

Any new rewards accrued since last claim (netRewardPerShare * balance)
Any existing unclaimedRewards amount from previous cycles

It then proceeds to:

1. Reset unclaimedRewards to 0

2. Emit RewardsClaimed event

3. Update totalRewardClaimed and per-user totals

4. Finally, it calls weth.safeTransfer() to send the pending rewards to the user

The critical issue is that steps 1-3 above occur before the rewards are transferred. This means if the weth.safeTransfer call fails for any reason, the user's unclaimedRewards balance is wiped. This permanently destroys any record of those pending rewards.

For example:

Alice has staked 10,000 TOKE for 1 year, accruing 100,000 WETH in unclaimedRewards
Alice calls collectRewards(), which calculates 50 WETH in new pending rewards
_collectRewards() resets Alice's unclaimedRewards to 0, losing the 100,000 WETH record
The weth.safeTransfer call fails due to gas limits
Alice has now permanently lost 100,000 WETH, worth over $100 million!

The contract meanwhile acts as if the rewards were successfully sent, creating a critical inconsistency in the platform's accounting.

This can occur any time the rewards transfer fails, due to:

Insufficient gas limits
External contract errors
Reentrancy/race conditions disrupting control flow
Temporary network issues

With no limit on unclaimed reward accrual over long durations, the potential loss amounts are essentially unlimited.

Impact

Users could lose all unclaimed rewards accrued over long stake durations
With no limit on reward accrual, losses could easily amount to millions of dollars worth of funds
Loss of user confidence, reputation damage, litigation risk
Permanent inconsistency in platform accounting and rewards totals
Requires complex manual reconciliation and user reimbursement

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/staking/GPToke.sol#L267C5-L316C6

    function _collectRewards(address user, bool distribute) internal returns (uint256) {
        // calculate user's new rewards per share (current minus claimed)
        uint256 netRewardsPerShare = accRewardPerShare - rewardDebtPerShare[user];
        // calculate amount of actual rewards
        uint256 netRewards = (balanceOf(user) * netRewardsPerShare) / REWARD_FACTOR;
        // get reference to user's pending (sandboxed) rewards
        uint256 pendingRewards = unclaimedRewards[user];

        // update checkpoint to current
        rewardDebtPerShare[user] = accRewardPerShare;

        // if nothing to claim, bail
        if (netRewards == 0 && pendingRewards == 0) {
            return 0;
        }

        if (distribute) {
            //
            // if asked for actual distribution, transfer all earnings
            //

            // reset sandboxed rewards
            unclaimedRewards[user] = 0;

            // get total amount by adding new rewards and previously sandboxed
            uint256 totalClaiming = netRewards + pendingRewards;

            // update running totals
            totalRewardsClaimed += totalClaiming;
            rewardsClaimed[user] += totalClaiming;

            emit RewardsClaimed(user, totalClaiming);

            // send rewards to user
            weth.safeTransfer(user, totalClaiming);

            // return total amount claimed
            return totalClaiming;
        }

        if (netRewards > 0) {
            // Save (sandbox) to their account for later transfer
            unclaimedRewards[user] += netRewards;

            emit RewardsCollected(user, netRewards);
        }

        // nothing collected
        return 0;
    }

Tool used

Manual Review

Recommendation

To address this, the unclaimedRewards reset and state changes should occur after the rewards transfer:


function _collectRewards() internal {

// Calculate rewards

// Transfer first

weth.safeTransfer(user, amount)

// State changes on success

unclaimedRewards[user] = 0;

emit RewardsClaimed(...)

totalRewardsClaimed += amount;

}

sherlock-audit / 2023-06-tokemak-judging

mrvincere - GPToke Rewards Collection Can Lead to Massive Loss of User Funds #3