Lines of code

Vulnerability details

Impact

The withdrawal amount will be counted as part of the surplus asset balance mistakenly if block.timestamp >= rewardsCycleEnd.

#Proof of Concept

    function beforeWithdraw(uint256 assets, uint256 shares) internal override {
        super.beforeWithdraw(assets, shares); // call xERC4626's beforeWithdraw first
        if (block.timestamp >= rewardsCycleEnd) { syncRewards(); } 
    }

storedTotalAssets will be deducted by the withdrawal amount in xERC4626.sol#beforeWithdraw().

At sfrxETH.sol#L50, when block.timestamp >= rewardsCycleEnd, syncRewards() will be called AFTER storedTotalAssets -= amount; (xERC4626.sol#L67).

<https://github.com/corddry/ERC4626/blob/6cf2bee5d784169acb02cc6ac0489ca197a4f149/src/xERC4626.sol#L65-L101&gt;

In syncRewards(), storedTotalAssets will be used to calculate the rewards.

> All surplus asset balance of the contract over the internal balance becomes queued for the next cycle.

uint256 storedTotalAssets_ = storedTotalAssets;
uint256 nextRewards = asset.balanceOf(address(this)) - storedTotalAssets_ - lastRewardAmount_;

storedTotalAssets = storedTotalAssets_ + lastRewardAmount_; // SSTORE

Tools Used

Recommended Mitigation Steps

Consider calling syncRewards() before call xERC4626’s beforeWithdraw.

The text was updated successfully, but these errors were encountered:

All reactions