sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

0x52 - WAuraPools will irreversibly break if reward tokens are added to pool after deposit #127

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

high

WAuraPools will irreversibly break if reward tokens are added to pool after deposit

Summary

WAuraPools will irreversibly break if reward tokens are added to pool after deposit due to an OOB error on accExtPerShare.

Vulnerability Detail

WAuraPools.sol#L166-L189

    uint extraRewardsCount = IAuraRewarder(crvRewarder)
        .extraRewardsLength(); <- @audit-issue rewardTokenCount pulled fresh
    tokens = new address[](extraRewardsCount + 1);
    rewards = new uint256[](extraRewardsCount + 1);

    tokens[0] = IAuraRewarder(crvRewarder).rewardToken();
    rewards[0] = _getPendingReward(
        stCrvPerShare,
        crvRewarder,
        amount,
        lpDecimals
    );

    for (uint i = 0; i < extraRewardsCount; i++) {
        address rewarder = IAuraRewarder(crvRewarder).extraRewards(i);

        @audit-issue attempts to pull from array which will be too small if tokens are added
        uint256 stRewardPerShare = accExtPerShare[tokenId][i];
        tokens[i + 1] = IAuraRewarder(rewarder).rewardToken();
        rewards[i + 1] = _getPendingReward(
            stRewardPerShare,
            rewarder,
            amount,
            lpDecimals
        );
    }

accExtPerShare stores the current rewardPerToken when the position is first created. It stores it as an array and only stores values for reward tokens that have been added prior to minting. This creates an issue if a reward token is added because now it will attempt to pull a value for an index that doesn't exist and throw an OOB error.

This is problematic because pendingRewards is called every single transaction via the isLiquidatable subcall in BlueBerryBank#execute.

Impact

WAuraPools will irreversibly break if reward tokens are added to pool after

Code Snippet

WAuraPools.sol#L152-L190

Tool used

Manual Review

Recommendation

Use a mapping rather than an array to store values

Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/dfe0479653f01453f468e58012bd0462f47eeed5#diff-ee660aa72b923bb7eb337255c5e07812c916ff2563f7351559fb634f0bf08cdf