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

Critical Vulnerability in _configureVaultConnectedMarkets::MarketMakingEngineConfigurationBranch: Arithmetic Underflow and Storage Corruption

heim

Summary

The _configureVaultConnectedMarkets function in the MarketMakingEngineConfigurationBranch contract contains a critical vulnerability that leads to arithmetic underflow and storage corruption. This occurs when the function attempts to access vault.connectedMarkets.length - 1 after failing to properly update the vault.connectedMarkets array. The issue allows an attacker to potentially manipulate the vault's connected markets or cause the contract to revert unexpectedly.

Vulnerability Details

The function _configureVaultConnectedMarkets is designed to configure connected markets for a vault. It performs the following steps:

1. Loads the vault data using Vault.load(vaultId).

2. Pushes a new array to vault.connectedMarkets.

  • Accesses the last element of the array using vault.connectedMarkets.length - 1.

  • Adds the provided market IDs to the new array.

However, the push() operation fails to update the storage, leaving the vault.connectedMarkets array empty. When the function attempts to access vault.connectedMarkets.length - 1, it causes an arithmetic underflow because length is 0.

Code snippet:

function _configureVaultConnectedMarkets(uint128 vaultId, uint256[] calldata marketsIds) internal {
// revert if vaultId is set to zero
if (vaultId == 0) {
revert Errors.ZeroInput("vaultId");
}
​
// load vault data from storage
Vault.Data storage vault = Vault.load(vaultId);
​
// push new array of connected markets
vault.connectedMarkets.push();
​
// use [vault.connectedMarkets.length - 1] to get the last connected markets array
uint256 connectedMarketsConfigIndex = vault.connectedMarkets.length - 1;
// add markets ids to connected markets
for (uint256 i; i < marketsIds.length; i++) {
vault.connectedMarkets[connectedMarketsConfigIndex].add(marketsIds[i]);
}
​
// emit event LogConfigureVaultConnectedMarkets
emit LogConfigureVaultConnectedMarkets(vaultId, marketsIds);
}

POC

  • Copy POC to path: test/audit-test/POCUncheckedArrayFuzzTest.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
​
import {Test} from "forge-std/Test.sol";
import {MarketMakingEngineConfigurationBranch} from "src/market-making/branches/MarketMakingEngineConfigurationBranch.sol";
import {Vault} from "@zaros/market-making/leaves/Vault.sol";
import {console} from "forge-std/console.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
​
// Derived contract to expose internal functions for testing
contract TestableMarketMakingEngineConfigurationBranch is MarketMakingEngineConfigurationBranch {
function testConfigureVaultConnectedMarkets(uint128 vaultId, uint256[] calldata marketsIds) external {
_configureVaultConnectedMarkets(vaultId, marketsIds);
}
}
​
contract POCUncheckedArrayFuzzTest is Test {
TestableMarketMakingEngineConfigurationBranch public engine;
address public owner = makeAddr("owner");
​
function setUp() public {
vm.startPrank(owner);
engine = new TestableMarketMakingEngineConfigurationBranch();
vm.stopPrank();
}
​
// Fuzz test: Handling random `marketsIds` arrays
function testFuzz_ConfigureVaultConnectedMarkets(uint128 vaultId, uint256[] calldata marketsIds) public {
// Ensure vaultId is not zero (as it would revert)
vm.assume(vaultId != 0);
// Constrain the array size to prevent gas limit issues
vm.assume(marketsIds.length <= 1000); // Limit array size to 1000
// Ensure all market IDs are valid (e.g., not zero)
for (uint256 i = 0; i < marketsIds.length; i++) {
vm.assume(marketsIds[i] != 0);
}
// Initialize the vault through engine functions only
// Log inputs for debugging
console.log("\n=== Test Inputs ===");
console.log("Testing function: _configureVaultConnectedMarkets");
console.log("Vault ID:", vaultId);
console.log("Number of Market IDs:", marketsIds.length);
console.log("Market IDs:");
for (uint256 i = 0; i < marketsIds.length; i++) {
console.log("Market ID[", i, "]:", marketsIds[i]);
}
// Log gas usage before the call
uint256 gasBefore = gasleft();
console.log("\n=== Gas Usage ===");
console.log("Gas before call:", gasBefore);
// Log the initial state of the vault
console.log("\n=== Initial Vault State ===");
Vault.Data storage vault = Vault.load(vaultId);
console.log("Initial connected markets length:", vault.connectedMarkets.length);
// Call the function
console.log("\n=== Function Call ===");
console.log("Calling _configureVaultConnectedMarkets...");
engine.testConfigureVaultConnectedMarkets(vaultId, marketsIds);
// Log gas usage after the call
uint256 gasAfter = gasleft();
console.log("\n=== Gas Usage ===");
console.log("Gas after call:", gasAfter);
console.log("Gas used:", gasBefore - gasAfter);
// Log the final state of the vault
console.log("\n=== Final Vault State ===");
console.log("Final connected markets length:", vault.connectedMarkets.length);
// Manually count the number of connected markets
uint256 count = 0;
for (uint256 i = 0; i < marketsIds.length; i++) {
if (EnumerableSet.contains(vault.connectedMarkets[vault.connectedMarkets.length - 1], marketsIds[i])) {
count++;
}
}
// Verify the function processed all market IDs
console.log("\n=== Verification ===");
console.log("Expected connected markets:", marketsIds.length);
console.log("Actual connected markets:", count);
assertEq(
count,
marketsIds.length,
"All markets should be added"
);
}
}

  • Run forge test --mc POCUncheckedArrayFuzzTest -vvv

  • Output:

=== Gas Usage ===
Gas before call: 1073573138
=== Initial Vault State ===
Initial connected markets length: 0
=== Function Call ===
Calling _configureVaultConnectedMarkets...
=== Gas Usage ===
Gas after call: 1069697559
Gas used: 3875579
=== Final Vault State ===
Final connected markets length: 0
=== Verification ===
Expected connected markets: 84
Actual connected markets: 0

Technical Analysis

1. Storage Update Failure

  • Test Output: The vault.connectedMarkets.length remains 0 after the function call, despite the function attempting to push a new array.

  • Root Cause: The push() operation in the function fails to persist the new array in storage. This could be due to:

  • Incorrect storage handling (e.g., modifying a local copy instead of the actual storage).

  • Missing state updates after the push() operation.

  • Implications: The vault.connectedMarkets array is not updated, rendering the function ineffective. This prevents the protocol from correctly configuring connected markets for the vault.

2. Arithmetic Underflow

  • Test Output: The function attempts to access vault.connectedMarkets.length - 1 when the array length is 0.

  • Root Cause: When vault.connectedMarkets.length is 0, the calculation length - 1 results in 2^256 - 1 due to underflow.

  • Implications: This underflow can lead to:

  • Accessing invalid memory locations, causing unexpected behavior.

  • Reverting the transaction, leading to a denial of service for legitimate users.

3. Impact on Functionality

  • Test Output: The function emits the LogConfigureVaultConnectedMarkets event, suggesting it executed successfully.

  • Root Cause: The event emission occurs before the underflow or storage update failure is detected.

  • Implications: The function appears to execute successfully, but the vault.connectedMarkets array remains unchanged. This creates a discrepancy between the expected and actual state of the vault, potentially leading to:

  • Incorrect vault configurations.

  • Financial losses for users relying on the protocol's functionality.

Impact

Severity: High

Impact on the Protocol

Direct Risk to Funds:

  • Incorrectly configured vaults could lead to financial losses for users, as the protocol may not function as intended.

  • Attackers could exploit the underflow to manipulate vault configurations, potentially draining funds or causing other financial harm.

Severe Disruption of Functionality:

  • The function fails to update the vault.connectedMarkets array, preventing the protocol from correctly configuring connected markets.

  • This could lead to inconsistencies in the vault's state, affecting other functions that rely on the connected markets.

Denial of Service:

  • If the function reverts due to the underflow, it could prevent legitimate users from configuring vaults, leading to a denial of service.

Likelihood of Exploitation

Straightforward Attack Path:

  • The vulnerability is easily exploitable, as it only requires calling the _configureVaultConnectedMarkets function with valid inputs.

  • No special conditions or complex steps are needed to trigger the underflow.
    High Probability of Exploitation:

  • The issue is present in a core function of the protocol, making it highly likely to be exploited if left unaddressed.

Tools Used

  • Foundry

Recommendations

Fix Storage Update:

  • Ensure vault.connectedMarkets.push() properly updates the storage.

  • Add checks to verify the array is not empty before accessing length - 1.

Updates

Lead Judging Commences

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

Support

FAQs

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