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

Potential Array Out-of-Bounds Access in marketIds and assets Assignment

0x0107

Summary

A vulnerability exists in the checkUpkeep function where the index variable could exceed the size of the preallocated arrays marketIds and assets, leading to an out-of-bounds memory access and contract failure.

Vulnerability Details

The vulnerability issue in checkUpKeep() function on FeeConversionKeeper.sol file

  • The arrays marketIds and assets are initialized with a fixed size of liveMarketIds.length * 10.

  • Inside the nested loop, the index variable is incremented without bounds checks.

  • If the combined iterations of the loops exceed the preallocated array size, this results in an out-of-bounds memory access, causing a runtime error.

PoC

function checkUpkeep(bytes calldata /\*\*/ ) external view returns
(bool upkeepNeeded, bytes memory performData) {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
​
// 1. Assuming liveMarketIds.length = 1 and marketAssets.length = 20:
uint128[] memory liveMarketIds = self.marketMakingEngine.getLiveMarketIds();
​
bool distributionNeeded;
uint128[] memory marketIds = new uint128[]();
address[] memory assets = new address[]();
uint256 index = 0;
uint128 marketId;
​
​
for (uint128 i; i < liveMarketIds.length; i++) {
marketId = liveMarketIds[i];
​
(address[] memory marketAssets, uint256[] memory feesCollected) =
self.marketMakingEngine.getReceivedMarketFees(marketId);
​
// 2.`marketAssets.length > marketIds.length`
for (uint128 j; j < marketAssets.length; j++) {
distributionNeeded = checkFeeDistributionNeeded(marketAssets[j], feesCollected[j]);
​
if (distributionNeeded) {
​
upkeepNeeded = true;
// 3.Out-of-bounds error
marketIds[index] = marketId;
assets[index] = marketAssets[j];
​
index++;
}
}
}
​
if (upkeepNeeded) {
performData = abi.encode(marketIds, assets);
}
}

Impact

The out-of-bounds error can lead to contract execution failure, disrupting operations dependent on checkUpkeep and potentially causing loss of service.

Tools Used

Manual Review

Recommendations

  1. Use dynamic array resizing:

    if (index == marketIds.length) {
    marketIds = resizeArray(marketIds, marketIds.length * 2);
    assets = resizeArray(assets, assets.length * 2);
    }

  2. Validate index before assignment to prevent exceeding the array's size.

  3. Precompute the maximum size required for arrays if feasible.

Updates

Lead Judging Commences

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.

Give us feedback!