DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Calculation not in favor of the protocol.

Summary

withdrawMarginUsd() has rounding errors in calculating the amount of withdrawal tokens.

Vulnerability Details

The relavant code of TradingAccount.withdrawMarginUsd() is following.

function withdrawMarginUsd(
Data storage self,
address collateralType,
UD60x18 marginCollateralPriceUsdX18,
UD60x18 amountUsdX18,
address recipient
)
internal
returns (UD60x18 withdrawnMarginUsdX18, bool isMissingMargin)
{
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
UD60x18 marginCollateralBalanceX18 = getMarginCollateralBalance(self, collateralType);
448: UD60x18 requiredMarginInCollateralX18 = amountUsdX18.div(marginCollateralPriceUsdX18);
uint256 amountToTransfer;
if (marginCollateralBalanceX18.gte(requiredMarginInCollateralX18)) {
452: withdraw(self, collateralType, requiredMarginInCollateralX18);
amountToTransfer =
marginCollateralConfiguration.convertUd60x18ToTokenAmount(requiredMarginInCollateralX18);
IERC20(collateralType).safeTransfer(recipient, amountToTransfer);
withdrawnMarginUsdX18 = amountUsdX18;
isMissingMargin = false;
} else {
withdraw(self, collateralType, marginCollateralBalanceX18);
amountToTransfer = marginCollateralConfiguration.convertUd60x18ToTokenAmount(marginCollateralBalanceX18);
IERC20(collateralType).safeTransfer(recipient, amountToTransfer);
withdrawnMarginUsdX18 = marginCollateralPriceUsdX18.mul(marginCollateralBalanceX18);
isMissingMargin = true;
}
}

As shown above, the function rounds down the requiredMarginInCollateralX18 in L448 and then it is used to withdraw collaterals in L452.
Since the requiredMarginInCollateralX18 is deducted from the user's balance, it should round up in order to be in favor of the protocol.

Impact

Although the amount of rounding error is small, withdrawMarginUsd() is called frequently in the protocol.
Calculating in a way that is not in favor of the protocol may cause a lack of collateral tokens to the protocol.

Tools Used

Manual Review

Recommendations

It is recommended to use rounding up in calculation.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

convertUd60x18ToTokenAmount heavily truncates

Support

FAQs

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