AMM price manipulation using openReversePosition
Description
The Nibiru perp module allows users to open reverse positions to decrease the margin, effectively shrinking the position size. A user can open a buy position and then immediately open a reverse position of the same size. Since currentPositionNotional is fractionally larger than notionalToDecreaseBy, it is possible to enter the decreasePostion flow as follows:
if currentPositionNotional.GT(notionalToDecreaseBy) {
// position reduction
return k.decreasePosition(
ctx,
market,
amm,
currentPosition,
notionalToDecreaseBy,
baseAmtLimit,
/* skipFluctuationLimitCheck */ false,This leaves the position with a zero size. Further in afterPositionUpdate, the position is not saved due to the following check:
func (k Keeper) afterPositionUpdate(
ctx sdk.Context,
market v2types.Market,
amm v2types.AMM,
traderAddr sdk.AccAddress,
positionResp v2types.PositionResp,
) (err error) {
[...]
if !positionResp.Position.Size_.IsZero() {
k.Positions.Insert(ctx, collections.Join(market.Pair, traderAddr), *positionResp.Position)
}However, the AMM is still updated in decreasePosition as though the position was saved.
func (k Keeper) decreasePosition(
ctx sdk.Context,
market v2types.Market,
amm v2types.AMM,
currentPosition v2types.Position,
decreasedNotional sdk.Dec,
baseAmtLimit sdk.Dec,
skipFluctuationLimitCheck bool,
) (updatedAMM *v2types.AMM, positionResp *v2types.PositionResp, err error) {
[...]
updatedAMM, baseAssetDeltaAbs, err := k.SwapQuoteAsset(
ctx,
market,
amm,
dir,
decreasedNotional,
baseAmtLimit,
)Impact
An attacker could repeatedly open and close positions to manipulate the AMM price. They could then liquidate strong positions to make a profit.
Recommendations
It appears that the afterPositionUpdate function does not update a position with size zero because it assumes that it has already been deleted — for example in closePositionEntirely:
positionResp.ExchangedNotionalValue = exchangedNotionalValue
positionResp.Position = &v2types.Position{
TraderAddress: currentPosition.TraderAddress,
Pair: currentPosition.Pair,
Size_: sdk.ZeroDec(),
Margin: sdk.ZeroDec(),
OpenNotional: sdk.ZeroDec(),
LatestCumulativePremiumFraction: market.LatestCumulativePremiumFraction,
LastUpdatedBlockNumber: ctx.BlockHeight(),
}
err = k.Positions.Delete(ctx, collections.Join(currentPosition.Pair, trader))Instead, a flag could be added to the PositionResp type to avoid updating a position after it has been deleted.
Remediation
This issue has been acknowledged by Nibiru, and fixes were implemented in the following commits: