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

Vaults that no longer delegate collateral but are included in credit capacity calculations

kwakudr

Summary

Previous market-vault connections aren’t explicitly removed when new ones are added. Existing configurations remain in storage, leading to outdated credit capacity calculations. Vaults/markets will reference obsolete connections, corrupting financial calculations. There is another issue of DOS due to continuous addition without removal.

Vulnerability Details

function _configureMarketConnectedVaults(uint128 marketId, uint256[] calldata vaultIds) internal {
// revert if vaultId is set to zero
if (marketId == 0) {
revert Errors.ZeroInput("marketId");
}
​
​
// load vault data from storage
Market.Data storage market = Market.load(marketId);
​
​
// if market has connected vaults
if (market.connectedVaults.length > 0) {
// Make sure that vaults connected to markets that are updated but not passed as an argument
uint256[] memory vaultIdToRecalculate = market.connectedVaults[market.connectedVaults.length - 1].values();
​
​
// recalculate the credit capacity for current market vaults
Vault.recalculateVaultsCreditCapacity(vaultIdToRecalculate);
}
​
​
// push new array of connected markets
market.connectedVaults.push();
​
​
// add markets ids to connected markets
for (uint256 i; i < vaultIds.length; i++) {
// use [vault.connectedVaults.length - 1] to get the last connected markets array
market.connectedVaults[market.connectedVaults.length - 1].add(vaultIds[i]);
}
​
​
// emit event LogConfigureMarketConnectedVaults
emit LogConfigureMarketConnectedVaults(marketId, vaultIds);
}
function connectVaultsAndMarkets(uint256[] calldata marketIds, uint256[] calldata vaultIds) external onlyOwner {
// revert if no marketIds are provided
if (marketIds.length == 0) {
revert Errors.ZeroInput("connectedMarketIds");
}
​
​
// revert if no vaultIds are provided
if (vaultIds.length == 0) {
revert Errors.ZeroInput("connectedVaultIds");
}
​
​
// define array that will contain a single vault id to recalculate credit for
uint256[] memory vaultIdToRecalculate = new uint256[](1);
​
​
// iterate over vault ids
for (uint128 i; i < vaultIds.length; i++) {
// if vault has connected markets
if (Vault.load(vaultIds[i].toUint128()).connectedMarkets.length > 0) {
// cache vault id
vaultIdToRecalculate[0] = vaultIds[i];
​
​
// recalculate the credit capacity of the vault
Vault.recalculateVaultsCreditCapacity(vaultIdToRecalculate);
}
}
​
​
for (uint256 i; i < marketIds.length; i++) {
_configureMarketConnectedVaults(marketIds[i].toUint128(), vaultIds);
}
​
​
// perform state updates for markets connected to each market id
for (uint256 i; i < vaultIds.length; i++) {
_configureVaultConnectedMarkets(vaultIds[i].toUint128(), marketIds);
}
}

The contract manages connections between markets and vaults via (connectVaultsAndMarkets) which Links a list of markets to a list of vaults by Looping through each market and calling _configureMarketConnectedVaults.

Inside _configureMarketConnectedVaults, the new vaults are added to the market’s connectedVaults array:

// In _configureMarketConnectedVaults:
market.connectedVaults.push(); // Adds a new set to the array
// Then adds new vaults to the latest set

Existing connections (old sets in connectedVaults) are never removed. When new vaults are linked to a market, the previous connections remain in storage. This leads to:

  • Credit recalculation (Vault.recalculateVaultsCreditCapacity) considering both old and new vaults, even when they’re no longer connected.

  • Another minor issue with the growing connections leading to DOS

Example:

  1. Initial State:

    • Market 1 connected to [Vault A, Vault B].

  2. Update Action:

    • Call connectVaultsAndMarkets([Market1], [Vault C, Vault D]).

  3. Result:

    • Market1.connectedVaults becomes:

      [
      Set([Vault A, Vault B]), // Old entry
      Set([Vault C, Vault D]) // New entry
      ]
      ​

      Both old (A, B) and new (C, D) vaults are tracked as "connected," causing recalculation logic to treat all four as active.

Impact

Vaults A and B no longer delegate collateral but are included in credit capacity calculations

Tools Used

Manual Review

Recommendations

// In _configureMarketConnectedVaults:
​
// Replace: Clear existing connectedVaults and replace with new list
if (market.connectedVaults.length > 0) {
// Delete older entries (assumes only one active config is kept)
delete market.connectedVaults;
}
​
// Push a new set with only the latest vault connections
market.connectedVaults.push();
for (uint256 i; i < vaultIds.length; i++) {
market.connectedVaults[market.connectedVaults.length - 1].add(vaultIds[i]);
}
Updates

Lead Judging Commences

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

Support

FAQs

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