Zellic - Audit Reports

Category: Coding Mistakes

Margin ratio not checked when removing collateral

Critical Severity

Critical Impact

High Likelihood

Description

When removing margin from a position using RemoveMargin, there is a check to ensure that there is enough free collateral:

func (k Keeper) RemoveMargin(
	ctx sdk.Context, pair asset.Pair, traderAddr sdk.AccAddress, marginToRemove sdk.Coin,
) (res *v2types.MsgRemoveMarginResponse, err error) {
	// fetch objects from state
	market, err := k.Markets.Get(ctx, pair)
	if err != nil {
		return nil, fmt.Errorf("%w: %s", types.ErrPairNotFound, pair)
	}

	amm, err := k.AMMs.Get(ctx, pair)
	if err != nil {
		return nil, fmt.Errorf("%w: %s", types.ErrPairNotFound, pair)
	}
	if marginToRemove.Denom != amm.Pair.QuoteDenom() {
		return nil, fmt.Errorf("invalid margin denom: %s", marginToRemove.Denom)
	}

	position, err := k.Positions.Get(ctx, collections.Join(pair, traderAddr))
	if err != nil {
		return nil, err
	}

	// ensure we have enough free collateral
	spotNotional, err := PositionNotionalSpot(amm, position)
	if err != nil {
		return nil, err
	}
	twapNotional, err := k.PositionNotionalTWAP(ctx, position, market.TwapLookbackWindow)
	if err != nil {
		return nil, err
	}
	minPositionNotional := sdk.MinDec(spotNotional, twapNotional)

	// account for funding payment
	fundingPayment := FundingPayment(position, market.LatestCumulativePremiumFraction)
	remainingMargin := position.Margin.Sub(fundingPayment)

	// account for negative PnL
	unrealizedPnl := UnrealizedPnl(position, minPositionNotional)
	if unrealizedPnl.IsNegative() {
		remainingMargin = remainingMargin.Add(unrealizedPnl)
	}

	if remainingMargin.LT(marginToRemove.Amount.ToDec()) {
		return nil, types.ErrFailedRemoveMarginCanCauseBadDebt.Wrapf(
			"not enough free collateral to remove margin; remainingMargin %s, marginToRemove %s", remainingMargin, marginToRemove,
		)
	}

	if err = k.Withdraw(ctx, market, traderAddr, marginToRemove.Amount); err != nil {
		return nil, err
	}

The issue is that there is no check to ensure that the new margin ratio of the position is valid and that it is not underwater.

Impact

This allows someone to open a new position and then immediately remove 99.99% of the margin while effectively allowing them to have infinite leverage.

Recommendations

There should be a check on the margin ratio, similar to afterPositionUpdate, to ensure that it is not too low:

var preferredPositionNotional sdk.Dec
if positionResp.Position.Size_.IsPositive() {
    preferredPositionNotional = sdk.MaxDec(spotNotional, twapNotional)
} else {
    preferredPositionNotional = sdk.MinDec(spotNotional, twapNotional)
}

marginRatio := MarginRatio(*positionResp.Position, preferredPositionNotional, market.LatestCumulativePremiumFraction)
if marginRatio.LT(market.MaintenanceMarginRatio) {
    return v2types.ErrMarginRatioTooLow
}

Remediation

This issue has been acknowledged by Nibiru, and a fix was implemented in commit ffad80c2↗.