sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Ch_301 - attackers will keep stealing the `rewards` from Convex SPELL #101

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

Ch_301

high

attackers will keep stealing the rewards from Convex SPELL

Summary

On WConvexPools.burn() transfer CRV + CVX + the extra rewards to Convex SPELL

Vulnerability Detail

But ConvexSpell.openPositionFarm() only refund CVX to the user. So the rest rewards will stay in the SPELL intel if someone (could be an attacker) invokes _doRefund() within closePositionFarm() with the same address tokens

Impact

Code Snippet

WConvexPools.burn() transfer CRV + CVX + the extra rewards https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/wrapper/WConvexPools.sol#L201-L235

        // Transfer LP Tokens
        IERC20Upgradeable(lpToken).safeTransfer(msg.sender, amount);

        // Transfer Reward Tokens
        (rewardTokens, rewards) = pendingRewards(id, amount);

        for (uint i = 0; i < rewardTokens.length; i++) {
            IERC20Upgradeable(rewardTokens[i]).safeTransfer(
                msg.sender,
                rewards[i]
            );
        }

only refund CVX to the user https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/spell/ConvexSpell.sol#LL127C1-L138C10

        // 6. Take out existing collateral and burn
        IBank.Position memory pos = bank.getCurrentPositionInfo();
        if (pos.collateralSize > 0) {
            (uint256 pid, ) = wConvexPools.decodeId(pos.collId);
            if (param.farmingPoolId != pid)
                revert Errors.INCORRECT_PID(param.farmingPoolId);
            if (pos.collToken != address(wConvexPools))
                revert Errors.INCORRECT_COLTOKEN(pos.collToken);
            bank.takeCollateral(pos.collateralSize);
            wConvexPools.burn(pos.collId, pos.collateralSize);
            _doRefundRewards(CVX);
        }

Tool used

Manual Review

Recommendation

you should Refund all Rewards (CRV + CVX + the extra rewards)

Ch-301 commented 1 year ago

Escalate for 10 USDC

Convex docs are confirming this point

Convex allows liquidity providers to earn trading fees and claim boosted CRV without locking CRV themselves. Liquidity providers can receive boosted CRV and liquidity mining rewards with minimal effort:
Earn claimable CRV with a high boost without locking any CRV
Earn CVX rewards
Zero deposit and withdraw fees
Zero fees on extra incentive tokens (SNX, etc)

and WConvexPools.burn() handle this properly

so Convex SPELL should refund all the rewards

sherlock-admin commented 1 year ago

Escalate for 10 USDC

Convex docs are confirming this point

Convex allows liquidity providers to earn trading fees and claim boosted CRV without locking CRV themselves. Liquidity providers can receive boosted CRV and liquidity mining rewards with minimal effort:
Earn claimable CRV with a high boost without locking any CRV
Earn CVX rewards
Zero deposit and withdraw fees
Zero fees on extra incentive tokens (SNX, etc)

and WConvexPools.burn() handle this properly

so Convex SPELL should refund all the rewards

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

ctf-sec commented 1 year ago

Senior watson's comment:

same as https://github.com/sherlock-audit/2023-05-blueberry-judging/issues/29

hrishibhat commented 1 year ago

Escalation accepted

Valid high This issue is a valid high along with another duplicate #42

sherlock-admin commented 1 year ago

Escalation accepted

Valid high This issue is a valid high along with another duplicate #42

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.
Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/3d981ac25709101e338fabeac54a489b28d0c3ac#diff-de367a2c91a38b5a20ed75683e2b8e0447531be4c07e5ac75207c5bb21f5f85a

IAm0x52 commented 1 year ago

Fixes look good. All tokens are now correctly returned to the user. PR above fixes ConvexSpell and this fixes AuraSpell