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

`Market::configureConnectedVaults` Will Always Fail with Array Out of Bounds Error

0xdarko

Summary

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/market-making/leaves/Market.sol#L381C1-L388C6

Market::configureConnectedVaults attempts to add vault IDs to a new enumerable set at index connectedVaults.length whithout first expanding the array's storage slots, causing an array out of bounds error.

Vulnerability Details

function configureConnectedVaults(Data storage self, uint128[] memory vaultsIds) internal {
EnumerableSet.UintSet[] storage connectedVaults = self.connectedVaults;
​
// Attempts to add to non-existent index
for (uint256 i; i < vaultsIds.length; i++) {
@> connectedVaults[connectedVaults.length].add(vaultsIds[i]);
}
}

For example, if connectedVaults has length 2 (indexes 0,1 exist), trying to add at connectedVaults[2] will revert because index 2 doesn't exist in storage yet. Arrays must be explicity expanded using push before accessing new indexes.

POC:
Add the following helper to MarketHarness.sol and update marketHarnessSelectors in TreeProxyUtils.sol to include it.

function exposed_configureConnectedVaults(uint128 marketId, uint128[] memory vaultIds) external {
Market.load(marketId).configureConnectedVaults(vaultIds);
}

Add this test to getAdjustedProfitForMarketId.t.sol

function test_outOfBounds(
uint256 marketId
) public {
MarketConfig memory config = getFuzzMarketConfig(marketId);
​
uint128[] memory vaultIds = new uint128[]();
vaultIds[0] = 1;
vaultIds[1] = 2;
// vm.expectRevert();
marketMakingEngine.exposed_configureConnectedVaults(config.marketId, vaultIds);
}

Impact

  • Function reverts with array out of bounds error

  • Any future upgrades or integrations using this function would fail

Tools Used

Foundry

Recommendations

Once grabbing connectedVaults, instead of just iterating and trying to add to connectedVaults.length:

  1. Call push on connectedVaults to expand the array to increase properly.

  2. Create a variable newSetIndex which will point to the newly created last slot in array

  3. Do the same for loop, but instead you are now pushing it to the proper expanded array index

function configureConnectedVaults(Data storage self, uint128[] memory vaultsIds) internal {
EnumerableSet.UintSet[] storage connectedVaults = self.connectedVaults;
​
- // add the vauls ids to a new UintSet instance in the connectedVaults array
- for (uint256 i; i < vaultsIds.length; i++) {
- connectedVaults[connectedVaults.length].add(vaultsIds[i]);
- }
+ connectedVaults.push();
+ uint256 newSetIndex = connectedVaults.length - 1;
​
// Add vault IDs to the new set
+ for (uint256 i; i < vaultsIds.length; i++) {
+ connectedVaults[newSetIndex].add(vaultsIds[i]);
+ }
​
}
Updates

Lead Judging Commences

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

`Market::configureConnectedVaults` Will Always Fail with Array Out of Bounds Error

Support

FAQs

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