Yield-Convex contest - kenzo's results

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).

Handle

kenzo

Vulnerability details

When querying Chainlink for stable prices, Cvx3CrvOracle doesn't run sanity checks against stale or incomplete results. This is unlike Yield's ChainlinkMultiOracle, which does execute those checks.

Impact

Stale or incorrect results might be returned.

Proof of Concept

When querying Chainlink, Cvx3CrvOracle only checks that the price is bigger than 0:

        (, int256 daiPrice, , , ) = DAI.latestRoundData();
        (, int256 usdcPrice, , , ) = USDC.latestRoundData();
        (, int256 usdtPrice, , , ) = USDT.latestRoundData();

        require(daiPrice > 0 && usdcPrice > 0 && usdtPrice > 0,
            "Chainlink pricefeed reporting 0");

But further checks are needed to verify the price is not stale, as Yield's ChainlinkMultiOracle does:

        (roundId, price,, updateTime, answeredInRound) = AggregatorV3Interface(source.source).latestRoundData();
        require(price > 0, "Chainlink price <= 0");
        require(updateTime != 0, "Incomplete round");
        require(answeredInRound >= roundId, "Stale price");

Recommended Mitigation Steps

Add the missing sanity checks.