Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Incorrect coversion of Liquidation Fee in `WithdrawMargin()` function in `TradingAccountBranch.sol`

viquetoh

Summary

An Issue stems in withdrawMargin() function which is UD60X18 type(usigned 60.18decimal fixed-point) to an SD59X18(signed 59.18). The current approach cast it to int128 first, which is risky because UD60X18 can represent values upto 1.15e59, but int128 can only go up 1.7e38. If liquidationFeeUsdX18 exceeds int128 max, this cast will overflow causing incorrect value or reverts.

Vulnerability Details

function withdrawMargin(uint128 tradingAccountId, address collateralType, uint256 amount) external {
// fetch storage slot for this collateral's config config
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
​
// load existing trading account; reverts for non-existent account
// enforces `msg.sender == owner` so only account owner can withdraw
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
​
// reverts if a trader has a pending order
_requireNoActiveMarketOrder(tradingAccountId);
​
// convert uint256 -> UD60x18; scales input amount to 18 decimals
UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount);
​
// enforce converted amount > 0
_requireAmountNotZero(amountX18);
​
// enforces that user has deposited enough collateral of this type to withdraw
_requireEnoughMarginCollateral(tradingAccount, collateralType, amountX18);
​
// deduct amount from trader's collateral balance
tradingAccount.withdraw(collateralType, amountX18);
​
// load account required initial margin requirement & unrealized USD profit/loss
// ignores "required maintenance margin" output parameter
(UD60x18 requiredInitialMarginUsdX18,, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
// get trader's margin balance
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
​
// check against initial margin requirement as initial margin > maintenance margin
// hence prevent the user from withdrawing all the way to the maintenance margin
// so that they couldn't be liquidated very soon afterwards if their position
// goes against them even a little bit
tradingAccount.validateMarginRequirement(requiredInitialMarginUsdX18, marginBalanceUsdX18, UD60x18_ZERO);
​
// cache whether the trader has an active position or not
bool userHasOpenPosition = !requiredInitialMarginUsdX18.isZero();
​
if (userHasOpenPosition) {
// load perps engine configuration from storage
PerpsEngineConfiguration.Data storage perpsEngineConfiguration = PerpsEngineConfiguration.load();
​
// save the trader's magin balance without taking the unrealized pnl into account
SD59x18 marginBalanceUsdWithoutUnrealizedPnlUsdX18 = tradingAccount.getMarginBalanceUsd(SD59x18_ZERO);
​
// verify if after the withdrawal the account will still own enough collateral to cover the liquidation
// fee value
if (
marginBalanceUsdWithoutUnrealizedPnlUsdX18.lt(
sd59x18(int128(perpsEngineConfiguration.liquidationFeeUsdX18))
)
) {
revert Errors.NotEnoughCollateralForLiquidationFee(perpsEngineConfiguration.liquidationFeeUsdX18);
}
}
​

Here the checks if the margin balance without unrealised PnL is less than the liquidation fee. The liquidation fee in the contract is stored as UD60X18 but when making comparisons it needs to be an SD59X18 to match margin balance type.

Impact

If the liquidationFeeUsdx18 exceeds int128's capacity the coversion overflows to a negative value. aalowing users to withdraw collateral even when their balance is insufficient.

Tools Used

Manual review

Recommendations

Replace unsafe cast with UD60X18 to SD59X18 conversion using PRBMaths library.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.