Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Deposits will exceed the intended cap in depositMargin

kwakudr

Summary

Deposits will exceed the intended cap.

Vulnerability Details

function depositMargin(uint128 tradingAccountId, address collateralType, uint256 amount) public virtual {
// 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
// does not enforce msg.sender == account owner so anyone can deposit
// collateral for other traders if they wish
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
​
​
// convert uint256 -> UD60x18; scales input amount to 18 decimals
UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount);
​
​
// uint128 -> UD60x18
UD60x18 depositCapX18 = ud60x18(marginCollateralConfiguration.depositCap);
​
​
// uint256 -> UD60x18
UD60x18 totalCollateralDepositedX18 = ud60x18(marginCollateralConfiguration.totalDeposited);
​
​
// enforce converted amount > 0
_requireAmountNotZero(amountX18);
​
​
// enforce new deposit + already deposited <= deposit cap
_requireEnoughDepositCap(collateralType, amountX18, depositCapX18, totalCollateralDepositedX18);
​
​
// enforce collateral has configured liquidation priority
_requireCollateralLiquidationPriorityDefined(collateralType);
​
​
// get the tokens first
IERC20(collateralType).safeTransferFrom(msg.sender, address(this), amount);
​
​
// then perform the actual deposit
tradingAccount.deposit(collateralType, amountX18);
​
​
emit LogDepositMargin(msg.sender, tradingAccountId, collateralType, amount);
}
UD60x18 depositCapX18 = ud60x18(marginCollateralConfiguration.depositCap);
​
​
// uint256 -> UD60x18
UD60x18 totalCollateralDepositedX18 = ud60x18(marginCollateralConfiguration.totalDeposited);
​
​
// enforce converted amount > 0
_requireAmountNotZero(amountX18);
​
​
// enforce new deposit + already deposited <= deposit cap
_requireEnoughDepositCap(collateralType, amountX18, depositCapX18, totalCollateralDepositedX18);

The error is in the handling of the deposit cap:

  1. The depositMargin function checks if the new deposit plus the already deposited amount is less than or equal to the deposit cap. However, it doesn't update the totalDeposited value after a successful deposit.

  2. This means that subsequent deposits will be checked against an outdated totalDeposited value, potentially allowing deposits to exceed the intended cap.

Impact

Deposits will exceed the intended cap.

Tools Used

Manual Review

Recommendations

`// Perform the check before the deposit
_requireEnoughDepositCap(collateralType, amountX18, depositCapX18, totalCollateralDepositedX18);

// Get the tokens
IERC20(collateralType).safeTransferFrom(msg.sender, address(this), amount);

// Perform the deposit
tradingAccount.deposit(collateralType, amountX18);

// Update the total deposited amount
marginCollateralConfiguration.totalDeposited = marginCollateralConfiguration.totalDeposited.add(amount);

emit LogDepositMargin(msg.sender, tradingAccountId, collateralType, amount);`

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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