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

[M-03] Array Overflow Leading to DoS

jakefromstatefarm

Summary

The FeeConversionKeeper contract contains a fixed-size array allocation vulnerability that can cause denial of service when processing markets with more than 10 assets.

Vulnerability Details

In the checkUpkeep function, the contract pre-allocates arrays with a fixed size assumption:

Line 75-76.

Code editor here doesn't like to show the full code:

uint128[] memory marketIds = new uint128[](); // (liveMarketIds.length * 10);
address[] memory assets = new address[](); // (liveMarketIds.length * 10);
(liveMarketIds.length * 10);
(liveMarketIds.length * 10);

Which assumes each market has at most 10 assets and is dangerous because there's no enforcement of this limit in the market creation process, the index variable can exceed array bounds if a market has >10 assets, and when the array bounds are exceeded, the entire upkeep operation fails.

for (uint128 i; i < liveMarketIds.length; i++) {
marketId = liveMarketIds[i];
(address[] memory marketAssets, uint256[] memory feesCollected) =
self.marketMakingEngine.getReceivedMarketFees(marketId);
// If marketAssets.length > 10, index will exceed array bounds
for (uint128 j; j < marketAssets.length; j++) {
if (distributionNeeded) {
marketIds[index] = marketId;
assets[index] = marketAssets[j];
index++;
}
}
}

Impact

I've rated this as MEDIUM because it can prevent fee conversion for markets when triggered, but it doesn't result in direct fund loss and there are potential workarounds like manual fee conversion. But

Recommendations

Add a constant maximum assets limit:

  • This ensures the limitation is enforced at all levels, preventing the DOS condition entirely.

function checkUpkeep(bytes calldata /**/ ) external view returns (bool upkeepNeeded, bytes memory performData) {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
​
uint128[] memory liveMarketIds = self.marketMakingEngine.getLiveMarketIds();
​
// Pre-allocate arrays with safe maximum size
uint128[] memory marketIds = new uint128[]();//fix here without (liveMarketIds.length * 10);
address[] memory assets = new address[](); //fix here without (liveMarketIds.length * 10);
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);
​
// Add safety check for maximum assets
if (marketAssets.length > MAX_ASSETS_PER_MARKET) {
revert TooManyMarketAssets(marketAssets.length, MAX_ASSETS_PER_MARKET);
}
​
// Iterate over receivedMarketFees
for (uint128 j; j < marketAssets.length; j++) {
bool distributionNeeded = checkFeeDistributionNeeded(marketAssets[j], feesCollected[j]);
Updates

Lead Judging Commences

inallhonesty Lead Judge
7 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.