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

Collateral deposits can exceed maximum cap for usdToken

AuditTemple

Summary

Redepositing of profits when updating market positions, leads to usd Token deposits increasing infinitely, bypassing the deposit cap.

Vulnerability Details

Existing trading accounts can deposit collateral through the TradingAccountBranch.depositMargin() function, which enforces a total cap that restricts the maximum global amount that can be deposited in the protocol for each collateral type (including the USDz token):

Reference to code: link

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);
​
....
​
// uint128 -> UD60x18
UD60x18 depositCapX18 = ud60x18(marginCollateralConfiguration.depositCap);
​
// uint256 -> UD60x18
UD60x18 totalCollateralDepositedX18 = ud60x18(marginCollateralConfiguration.totalDeposited);
​
....
​
// enforce new deposit + already deposited <= deposit cap
_requireEnoughDepositCap(collateralType, amountX18, depositCapX18, totalCollateralDepositedX18);
​
....
// then perform the actual deposit
tradingAccount.deposit(collateralType, amountX18);
....
}
​
​

After the cap is reached, deposits revert - this way the protocol control it’s exposure to particular assets.

However when an existing market position gets updated with a new market order through the SettlementBranch._fillOrder() function, any accrued Profit in the USDz token before the update is automatically re-deposited without considering any deposit cap limitations:

Reference to code: link

function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
internal
virtual
{
...
​
// if trader's old position had positive pnl then credit that to the trader
if (ctx.pnlUsdX18.gt(SD59x18_ZERO)) {
ctx.marginToAddX18 = ctx.pnlUsdX18.intoUD60x18();
@> tradingAccount.deposit(ctx.usdToken, ctx.marginToAddX18);
​
// mint settlement tokens credited to trader; tokens are minted to
// address(this) since they have been credited to trader's deposited collateral
//
// NOTE: testnet only - this call will be updated once the Market Making Engine is finalized
LimitedMintingERC20(ctx.usdToken).mint(address(this), ctx.marginToAddX18.intoUint256());
}
...
}
​

This basically means that as long as there are profits, the deposits will keep on increasing constantly, rendering the deposit cap inefficient.

In case there are a lot of USDz positions and a lot of them make profits, the total deposits can go way above the expected cap by the protocol and significantly hinder it’s mechanism of regulating and balancing the markets

Impact

USDz deposits will go above the expected maximum cap

Tools Used

Manual Review

Recommended Mitigation

Since _fillOrder is a function that should not revert due to max limits being reached it is understandable that the max cap is not checked here.

A possible approach would be to send the funds to the account owner instead of re-depositing them. This will prevent distortion of the maximum caps and required collateral exposures by the protocol.

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

ilchovski Submitter
10 months ago
b0g0 Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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