Zellic - Audit Reports

Shares can round down to zero

The proRataRedeem function allows users to redeem their shares in the basket when the basket is not in the rebalancing state. The function calculates the amount of each asset to be redeemed based on the number of shares to be redeemed and the total supply of the basket. The function then transfers the calculated amount of each asset to the user. However, the function does not revert if the calculated amount of an asset is zero.

function proRataRedeem(
    BasketManagerStorage storage self,
    uint256 totalSupplyBefore,
    uint256 burnedShares,
    address to
)
    external
{
    // Checks
    if (totalSupplyBefore == 0) {
        revert ZeroTotalSupply();
    }
    if (burnedShares == 0) {
        revert ZeroBurnedShares();
    }
    if (burnedShares > totalSupplyBefore) {
        revert CannotBurnMoreSharesThanTotalSupply();
    }
    if (to == address(0)) {
        revert Errors.ZeroAddress();
    }
    // Revert if the basket is currently rebalancing
    if ((self.rebalanceStatus.basketMask & (1 << self.basketTokenToIndexPlusOne[msg.sender] - 1)) != 0) {
        revert MustWaitForRebalanceToComplete();
    }

    address basket = msg.sender;
    address[] storage assets = self.basketAssets[basket];
    uint256 assetsLength = assets.length;

    // Interactions
    for (uint256 i = 0; i < assetsLength;) {
        address asset = assets[i];
        // nosemgrep: solidity.performance.state-variable-read-in-a-loop.state-variable-read-in-a-loop
        uint256 balance = self.basketBalanceOf[basket][asset];
        // Rounding direction: down
        // Division-by-zero is not possible: totalSupplyBefore is greater than 0
        uint256 amountToWithdraw = FixedPointMathLib.fullMulDiv(burnedShares, balance, totalSupplyBefore);
        if (amountToWithdraw > 0) {
            // nosemgrep: solidity.performance.state-variable-read-in-a-loop.state-variable-read-in-a-loop
            self.basketBalanceOf[basket][asset] = balance - amountToWithdraw;
            // Asset is an allowlisted ERC20 with no reentrancy problem in transfer
            // slither-disable-next-line reentrancy-no-eth
            IERC20(asset).safeTransfer(to, amountToWithdraw);
        }
        unchecked {
            // Overflow not possible: i is less than assetsLength
            ++i;
        }
    }
}

This can lead to a situation where the user redeems shares but receives no assets in return. This is because the burnedShares * balance / totalSupplyBefore calculation can round down to zero when burnedShares is small.

Due to lack of an adequate fix for the current code architecture, we leave this section for potential future consideration, should the code allow for a definite fix. As of the time of writing, a revert in the case of amountToWithdraw could be problematic for users. It may be that not all of the assets are redeemable at a rate greater than zero, but the user should still be able to redeem the ones that are. To prevent the price of the share token from inflating, the initial asset must be deposited after creating the basket token. Storm Labs can refer to this GitHub issue↗, "Implement or recommend mitigations for ERC4626 inflation attacks", for typical mitigations for ERC-4626 vault inflation.