code-423n4 / 2021-12-yetifinance-findings

0 stars 0 forks source link

StabilityPool does not update rewards when upwrapping wrapped asset #152

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

kenzo

Vulnerability details

When StabilityPool sends funds to depositor using _sendGainsToDepositor (which is called from provideToSP or withdrawFromSP), the user's wrapped asset's rewards are not being updated.

Impact

Incorrect yield accounting.

Proof of Concept

_sendGainsToDepositor calls a wrapped asset's unwrapFor, but does not update the rewards for the depositor: (Code ref)

        for (uint256 i = 0; i < assets.length; i++) {
            if (whitelist.isWrapped(assets[i])){
                IWAsset(assets[i]).endTreasuryReward(amounts[i]);
                IWAsset(assets[i]).unwrapFor(_to, amounts[i]);
            } else {
                IERC20(assets[i]).transfer(_to, amounts[i]);
            }
        }

This means that the user will continue to accrue rewards even though the asset is out of the system.

Recommended Mitigation Steps

Add a call to the wrapped asset's claimRewardFor before unwrapping, or call _userUpdate in the beginning of unwrapFor to make sure the rewards are always up to date. This is what wrap does.

kingyetifinance commented 2 years ago

@LilYeti: So here before it is sent to the stability pool, the rewards are redirected to the treasury. This is because it is difficult to keep track of rewards for all stability pool stakers for various JLP tokens that are generating the same JOE, for instance, so while they have the WJLP claimed, it no longer generates yield for them, and rewards are directed to the treasury instead. Due to that, it calls end treasury reward when removing from stability pool instead.

alcueca commented 2 years ago

Downgraded to code clarity issue.