020.md

Small Hazel Lemur

Medium

_ineligible() inside redemptionEligible is slightly miscalculated

Summary

_ineligible() inside redemptionEligible is slightly miscalculated

Root Cause

claimAmount doesn't equal claimAssets, so whenever eligible redemption is calculated to get latest global assets before withdrawal, there should be addition of claimAssets, which is happening in context.global.update before calling _manage

        // asses socialization
        UFixed6 claimAmount = _socialize(context, claimAssets);

        // update positions
->        context.global.update(context.currentId, claimAssets, redeemShares, depositAssets, redeemShares);
        context.local.update(context.currentId, claimAssets, redeemShares, depositAssets, redeemShares);
        context.currentCheckpoint.update(depositAssets, redeemShares);

        // manage assets
        asset.pull(msg.sender, UFixed18Lib.from(depositAssets));
->        _manage(context, depositAssets, claimAmount, !depositAssets.isZero() || !redeemShares.isZero());
        asset.push(msg.sender, UFixed18Lib.from(claimAmount));

vault/contracts/Vault.sol#L463

similar to issue before

Internal pre-conditions

External pre-conditions

Attack Path

Impact

One of the main effects of _ineligible() is that this part cannot be used as an asset to open a position; if this value is too small, too many positions are opened, resulting in the inability to claimAssets properly.

PoC

Mitigation

use claimAssets to get context.global.assets before withdrawal

-    function _manage(Context memory context, UFixed6 deposit, UFixed6 withdrawal, bool shouldRebalance) private {
+    function _manage(Context memory context, UFixed6 deposit, UFixed6 withdrawal, bool shouldRebalance, UFixed6 claimAssets) private {
        if (context.totalCollateral.lt(Fixed6Lib.ZERO)) return;

-        Target[] memory targets = _strategy(context, deposit, withdrawal, _ineligible(context, deposit, withdrawal));
+        Target[] memory targets = _strategy(context, deposit, withdrawal, _ineligible(context, deposit, claimAssets));

        for (uint256 marketId; marketId < context.registrations.length; marketId++)
            if (targets[marketId].collateral.lt(Fixed6Lib.ZERO))
                _retarget(context.registrations[marketId], targets[marketId], shouldRebalance);
        for (uint256 marketId; marketId < context.registrations.length; marketId++)
            if (targets[marketId].collateral.gte(Fixed6Lib.ZERO))
                _retarget(context.registrations[marketId], targets[marketId], shouldRebalance);
    }