Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sherlock-admin4 opened this issue Apr 25, 2024 · 3 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Apr 25, 2024

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.

@github-actions github-actions bot closed this as completed May 5, 2024
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 5, 2024
@sherlock-admin4
Copy link
Contributor Author

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
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
Zivoe/zivoe-core-foundry#273

@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels May 24, 2024
@sherlock-admin2
Copy link
Contributor

The Lead Senior Watson signed off on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants