Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: medium
Valid

Invalid entries may be encoded into performData and later decoded in `performUpkeep`, potentially causing the `convertAccumulatedFeesToWeth` function to revert

Summary

The checkUpkeep function in the FeeConversionKeeper contract pre-allocates a fixed-size array (marketIds) to store market IDs that require fee distribution. However, this array may not be fully populated, leading to empty or invalid entries (e.g., 0). These invalid entries are encoded into performData and later decoded in performUpkeep, potentially causing the convertAccumulatedFeesToWeth function to be called with invalid parameters, such as a marketId of 0. This could result in failed transactions, wasted gas, or unintended behavior in the MarketMakingEngine contract.

Vulnerability Details

In checkUpkeep, the marketIds and assets arrays are initialized with a fixed size (liveMarketIds.length * 10), which is significantly larger than the actual number of valid entries.

function checkUpkeep(bytes calldata /**/ ) external view returns (bool upkeepNeeded, bytes memory performData) {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
uint128[] memory liveMarketIds = self.marketMakingEngine.getLiveMarketIds();
bool distributionNeeded;
uint128[] memory marketIds = new uint128[]();
address[] memory assets = new address[]();
uint256 index = 0;
uint128 marketId;
...
...

Unused slots in these arrays are filled with default values (0 for uint128 and address(0) for addresses).

The marketIds and assets arrays, including their empty or invalid entries, are encoded into performData and passed to performUpkeep.

if (upkeepNeeded) {
performData = abi.encode(marketIds, assets);
}
// call FeeDistributionBranch::convertAccumulatedFeesToWeth
function performUpkeep(bytes calldata performData) external override onlyForwarder {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
IMarketMakingEngine marketMakingEngine = self.marketMakingEngine;
// decode performData
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
// convert accumulated fees to weth for decoded markets and assets
for (uint256 i; i < marketIds.length; i++) {
marketMakingEngine.convertAccumulatedFeesToWeth(marketIds[i], assets[i], self.dexSwapStrategyId, "");
}
}

In performUpkeep, performData is decoded into marketIds and assets arrays. If these arrays contain invalid entries (e.g., marketId = 0), the convertAccumulatedFeesToWeth function may revert.

Impact

The impact is High because the convertAccumulatedFeesToWeth function will revert, the likelihood is Medium, so the severity is High.

Tools Used

Manual Review

Recommendations

Consider adding validation in performUpkeep

function performUpkeep(bytes calldata performData) external override onlyForwarder {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
IMarketMakingEngine marketMakingEngine = self.marketMakingEngine;
// Decode performData
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
// Validate array lengths
require(marketIds.length == assets.length, "Invalid performData: array length mismatch");
for (uint256 i; i < marketIds.length; i++) {
uint128 marketId = marketIds[i];
address asset = assets[i];
// Skip invalid entries
if (marketId == 0 || asset == address(0)) {
emit InvalidMarketId(marketId, asset); // Log invalid entries
continue;
}
// Call convertAccumulatedFeesToWeth
marketMakingEngine.convertAccumulatedFeesToWeth(marketId, asset, self.dexSwapStrategyId, "");
}
}
Updates

Lead Judging Commences

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

FeeConversionKeeper performUpkeep iterates over entire oversized arrays instead of just populated entries, causing reverts on address(0) slots

Support

FAQs

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