Summary
The SettlementBranch._fillOrder function credits more tokens to the trader than minted to the contract. This happens because the adjusted value goes to the withdrawUsdTokenFromMarket function and then it is adjusted again there.
Vulnerability Details
The positive pnl is adjusted when the market is in the ADL state. Then the received value is credited to the tradingAccount. But the same value goes to the withdrawUsdTokenFromMarket where it is adjusted again. So the minted value can be less than credited.
function _fillOrder(
<...>
if (ctx.pnlUsdX18.gt(SD59x18_ZERO)) {
IMarketMakingEngine marketMakingEngine = IMarketMakingEngine(perpsEngineConfiguration.marketMakingEngine);
>> ctx.marginToAddX18 =
marketMakingEngine.getAdjustedProfitForMarketId(marketId, ctx.pnlUsdX18.intoUD60x18().intoUint256());
tradingAccount.deposit(perpsEngineConfiguration.usdToken, ctx.marginToAddX18);
>> marketMakingEngine.withdrawUsdTokenFromMarket(marketId, ctx.marginToAddX18.intoUint256());
}
CreditDelegationBranch.sol
function withdrawUsdTokenFromMarket(uint128 marketId, uint256 amount) external onlyRegisteredEngine(marketId) {
<...>
UD60x18 amountX18 = ud60x18(amount);
>> uint256 amountToMint = amount;
if (market.isAutoDeleverageTriggered(delegatedCreditUsdX18, marketTotalDebtUsdX18)) {
UD60x18 adjustedUsdTokenToMintX18 =
market.getAutoDeleverageFactor(delegatedCreditUsdX18, marketTotalDebtUsdX18).mul(amountX18);
>> amountToMint = adjustedUsdTokenToMintX18.intoUint256();
market.updateNetUsdTokenIssuance(adjustedUsdTokenToMintX18.intoSD59x18());
} else {
market.updateNetUsdTokenIssuance(amountX18.intoSD59x18());
}
MarketMakingEngineConfiguration.Data storage marketMakingEngineConfiguration =
MarketMakingEngineConfiguration.load();
UsdToken usdToken = UsdToken(marketMakingEngineConfiguration.usdTokenOfEngine[msg.sender]);
usdToken.mint(msg.sender, amountToMint);
emit LogWithdrawUsdTokenFromMarket(msg.sender, marketId, amount, amountToMint);
}
Impact
Discrepancy in internal balances can cause different problems like assets losses, lack of collateralize, incorrect calculations, breaking invariant, DoS of functionality
Tools used
Manual Review
Recommendations
Consider using not adjusted value in the withdrawUsdTokenFromMarket call:
// if trader's old position had positive pnl then credit that to the trader
if (ctx.pnlUsdX18.gt(SD59x18_ZERO)) {
IMarketMakingEngine marketMakingEngine = IMarketMakingEngine(perpsEngineConfiguration.marketMakingEngine);
ctx.marginToAddX18 =
marketMakingEngine.getAdjustedProfitForMarketId(marketId, ctx.pnlUsdX18.intoUD60x18().intoUint256());
tradingAccount.deposit(perpsEngineConfiguration.usdToken, ctx.marginToAddX18);
// mint settlement tokens credited to trader; tokens are minted to
// address(this) since they have been credited to the trader's margin
- marketMakingEngine.withdrawUsdTokenFromMarket(marketId, ctx.marginToAddX18.intoUint256());
+ marketMakingEngine.withdrawUsdTokenFromMarket(marketId, ctx.pnlUsdX18);
}
