LiquidationModule.liquidate updates global position data with stale price

Summary

[LiquidationModule.liquidate](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LiquidationModule.sol#L85) is used to liquidate a position. Within this function, after [the position is settled](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LiquidationModule.sol#L109-L143), [vault.updateGlobalPositionData will be called to update the global position data to the latest status](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LiquidationModule.sol#L159-L163). However, the current implementation incorrectly passes the lastPrice of the liquidated position as the fresh price to vault.updateGlobalPositionData, causing a fatal error in the global position data.

Vulnerability Detail

File: flatcoin-v1\src\LiquidationModule.sol
085:     function liquidate(uint256 tokenId) public nonReentrant whenNotPaused liquidationInvariantChecks(vault, tokenId) {
86:         FlatcoinStructs.Position memory position = vault.getPosition(tokenId);
......
088:->       (uint256 currentPrice, ) = IOracleModule(vault.moduleAddress(FlatcoinModuleKeys._ORACLE_MODULE_KEY)).getPrice();
......//check whether the position can liquidated, then settle postion
145:         // Update the global position data.
......//dev's comment
159:         vault.updateGlobalPositionData({
160:->           price: position.lastPrice,
161:             marginDelta: -(int256(position.marginDeposited) + positionSummary.accruedFunding),
162:             additionalSizeDelta: -int256(position.additionalSize) // Since position is being closed, additionalSizeDelta should be negative.
163:         });
......
177:     }

L86, get the position information to be liquidated.

L87-L158, which checks whether the position can be liquidated. If not, tx revert. If yes, settle the P&L of the position and the liquidator's fee.

L159-L163, vault.updateGlobalPositionData is used to settle the global total profit and loss. Its first parameter, price, is the key to calculating profit and loss. The problem lies here, it should be a fresh price, not the lastPrice of the liquidated position.

position.lastPrice is equivalent to:

If the position has never been adjusted after it was opened, then lastPrice is the entry price of the position.
If the position has been adjusted before being liquidated, then lastPrice is the price obtained from oracle when the position was last adjusted.

This means that lastPrice is stale.

File: flatcoin-v1\src\FlatcoinVault.sol
173:     function updateGlobalPositionData(
174:         uint256 _price,
175:         int256 _marginDelta,
176:         int256 _additionalSizeDelta
177:     ) external onlyAuthorizedModule {
178:         // Get the total profit loss and update the margin deposited total.
179:->       int256 profitLossTotal = PerpMath._profitLossTotal({globalPosition: _globalPositions, price: _price});
......//dev's comment
184:->       int256 newMarginDepositedTotal = int256(_globalPositions.marginDepositedTotal) + _marginDelta + profitLossTotal;
......//dev's comment
191:         if (newMarginDepositedTotal < 0) {
192:             revert FlatcoinErrors.InsufficientGlobalMargin();
193:         }
194: 
195:         _globalPositions = FlatcoinStructs.GlobalPositions({
196:->           marginDepositedTotal: uint256(newMarginDepositedTotal),
197:             sizeOpenedTotal: (int256(_globalPositions.sizeOpenedTotal) + _additionalSizeDelta).toUint256(),
198:->           lastPrice: _price
199:         });
......//dev's comment
205:->       _updateStableCollateralTotal(-profitLossTotal);
206:     }

L179, profitLossTotal is calculated by the formula globalPosition.sizeOpenedTotal * (int256(price) - int256(globalPosition.lastPrice)) / int256(price). If price is a stale price, profitLossTotal will be incorrect.

L184, simple calculates newMarginDepositedTotal.

L195-199, update _globalPositions using the values calculated above.

L205, update stableCollateralTotal for stable LP.

Consider the following scenario:

Alice opens a position A, and the entry price is 1000e18, that is, positionA.lastPrice = 1000e18.
As time passes, Position A can be liquidated. The current price is 799e18. Notice that _globalPositions.lastPrice is the price when some user's order was last executed. We can think of it as close to the current price, here for simplicity let's assume it is 800e18.
A liquidator calls liquidate for position A. Because vault.updateGlobalPositionData uses positionA.lastPrice to update global position data. At L179, profitLossTotal will be equal to globalPosition.sizeOpenedTotal * (1000e18 - 800e18) / 1000e18.

Impact

This can have a significant impact:

_globalPositions.marginDepositedTotal will increase significantly (for long party), while stableCollateralTotal will decrease significantly (for short party) and the worst case may be 0. stableCollateralTotal determines the amount of collateral that stable LP can withdraw. The smaller the stableCollateralTotal, the smaller the amount of collateral that can be withdrawn. Stable LP will suffer significant funds losses.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/LiquidationModule.sol#L160

Tool used

Manual Review

Recommendation

Call vault.updateGlobalPositionData with the fresh price.

Duplicate of #188

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid; high(1)