sherlock-audit / 2024-03-zivoe-judging

1 stars 0 forks source link

BoRonGod - cannot forward extra rewards from both OCY_Convex to OCT_YDL. #479

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

BoRonGod

high

cannot forward extra rewards from both OCY_Convex to OCT_YDL.

Summary

Convex specifies rewardContract to be a VirtualBalanceRewardPool, but all three OCY_Convex uses it as a ERC20 token, which make it impossible to claim extra rewards and forward them to the OCT_YDL.

Vulnerability Detail

According to Convex doc:

The BaseRewardPool has an array of child reward contracts called extraRewards. You can query the number of extra rewards via baseRewardPool.extraRewardsLength(). This array holds a list of VirtualBalanceRewardPool contracts which are similar in nature to the base reward contract but without actual control of staked tokens.

This means that if a pool has CRV rewards as well as SNX rewards, the pool's main reward contract(BaseRewardPool) will distribute the CRV and the child contract(VirtualBalanceRewardPool) will distribute the SNX.

But, in current implementation: (take OCY_Convex_A for example)

    // Extra Rewards
    if (extra) {
        uint256 extraRewardsLength = IBaseRewardPool_OCY_Convex_A(convexRewards).extraRewardsLength();
        for (uint256 i = 0; i < extraRewardsLength; i++) {
            address rewardContract = IBaseRewardPool_OCY_Convex_A(convexRewards).extraRewards(i);
            uint256 rewardAmount = IBaseRewardPool_OCY_Convex_A(rewardContract).rewardToken().balanceOf(
                address(this)
            );
            //@Audit rewardContract is a VirtualBalanceRewardPool
            if (rewardAmount > 0) { IERC20(rewardContract).safeTransfer(OCT_YDL, rewardAmount); }
        }
    }

Tokens cannot be sent to YDL.

Impact

Current OCY_Convex_A, OCY_Convex_B and OCY_Convex_C cannot forward extraRewards to YDL.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/lockers/OCY/OCY_Convex_A.sol#L263 https://docs.convexfinance.com/convexfinanceintegration/baserewardpool#extra-rewards

Tool used

Manual Review

Recommendation

Use the real token address for token transfer.

sherlock-admin4 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

high, dup of #477, loss of additional reward from convex. Since it accumulates over 30-days period, the loss will be significant. Both balanceOf and safeTransfer are incorrect - they should be called on rewardToken().token(). Different dups of this mention either balanceOf or safeTransfer, but not both, but I consider them to be the same root cause, so all are dups.

sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/Zivoe/zivoe-core-foundry/pull/273

sherlock-admin2 commented 1 month ago

The Lead Senior Watson signed off on the fix.