Blueberryfi / blueberry-core

MIT License
7 stars 5 forks source link

Audit Ch_301: [H-01] Users will lose some of their rewards favor to attacker #64

Closed Ch-301 closed 1 year ago

Ch-301 commented 1 year ago

[H-01] Users will lose some of their rewards favor to attacker

Severity

Impact: High

Probability: Unlikely

Context

AuraSpell.sol

Description

In case the contract of auraRewarder was updated with additional new tokens in their extraRewards[ ] (or even it didn't have any extraRewards at the beginning).

In this part _syncExtraReward() function will be triggered to add the new tokens to extraRewards[ ], so users could be qualified to receive rewards in these tokens

Now the impact: In the step 6 Take out existing collateral and burn

File: AuraSpell.sol

        // 6. Take out existing collateral and burn
        IBank.Position memory pos = bank.getCurrentPositionInfo();
        if (pos.collateralSize > 0) {
            (uint256 pid, ) = wAuraPools.decodeId(pos.collId);
            if (param.farmingPoolId != pid)
                revert Errors.INCORRECT_PID(param.farmingPoolId);
            if (pos.collToken != address(wAuraPools))
                revert Errors.INCORRECT_COLTOKEN(pos.collToken);
            (address[] memory rewardTokens, ) = IERC20Wrapper(pos.collToken)
                .pendingRewards(pos.collId, pos.collateralSize);
            bank.takeCollateral(pos.collateralSize);
            wAuraPools.burn(pos.collId, pos.collateralSize);
            // distribute multiple rewards to users
            uint256 rewardTokensLength = rewardTokens.length;
            for (uint256 i; i != rewardTokensLength; ) {
                _doRefundRewards(
                    rewardTokens[i] == STASH_AURA ? AURA : rewardTokens[i]
                );
                unchecked {
                    ++i;
                }
            }
        }

by invoking pendingRewards() befor burn() function You are excluding the user from receiving his awards because rewardTokens[ ] array isn't complete with all the extraRewards address.

This will allow any attacker to withdraw these rewards so the users lossing funds (Rewards)

Recommendations

You need to postpone this line until the sub-call of burn()

Ch-301 commented 1 year ago

The fix looks good. Now rewardTokens[ ] comes directly from wAuraPools.burn()

https://github.com/Blueberryfi/blueberry-core/blob/929cb60417924d51f68155432fc0e87a4dc2ef40/contracts/spell/AuraSpell.sol#L148-L151