ConvexStakingWrapper does not update rewards state before transferring tokens

[code423n4]

Handle

kenzo

Vulnerability details

ConvexStakingWrapper saves data for reward calculation in dedicated variables for each user, such as reward.reward_integral_for[account]. These variables are not updated when transferring wrapped staked tokens. (Please note that Convex's original ConvexStakingWrapper does update the rewards state before transfer.)

Impact

Wrong accounting for rewards.

Proof of Concept

Let's say Roy hasn't claimed his rewards, and transfers tokens to Zora. Next time Roy will call getReward, it will call the _checkpointAndClaim method, which will call _getDepositedBalance to calculate Roy's balance. Since his balance is now 0, and he didn't have already saved pending rewards (as no checkpoint was created when transferring the tokens), the function will calculate that he has no pending rewards. His rewards have been lost in time, like tears in rain.

Recommended Mitigation Steps

Add the following code, like Convex does:

    function _beforeTokenTransfer(address _from, address _to, uint256 _amount) internal override {
        _checkpoint([_from, _to]);
    }

(Note: you can then remove the _checkpoint calls from wrap and unwrap).

[iamsahu]

The solution provided won't work. The _checkpoint before a token transfer needs to be conditional otherwise it would end up allocating the wrapper contract some of the rewards.

[alcueca]

Still, the issue is valid. Duplicate of #86, which we agreed to be Sev 3.