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

performUpkeep will always revert in case of fee distribution

0xasad97

Summary

The function performUpkeep in the FeeConversionKeeper contract always reverts due to an improper memory allocation issue in checkUpkeep. Specifically, the code over-allocates memory for the marketIds and assets arrays.

Vulnerability Details

The arbitrary 10 multiplication leads to uninitialized memory slots, causing invalid data access. performUpkeep attempts to process these uninitialized indexes, leading to a revert.
The error arises when trying to access totalAssets() on an uninitialized zero address, leading to a revert.

/src/external/chainlink/keepers/fee-conversion-keeper/FeeConversionKeeper.sol:69
69: function checkUpkeep(bytes calldata /**/ ) external view returns (bool upkeepNeeded, bytes memory performData) {
70: FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
71:
72: uint128[] memory liveMarketIds = self.marketMakingEngine.getLiveMarketIds();
73:
74: bool distributionNeeded;
75: uint128[] memory marketIds = new uint128[](liveMarketIds.length * 10);
76: address[] memory assets = new address[](liveMarketIds.length * 10);

Consider the following Proof of code:

POC

/test/integration/external/chainlink/keepers/fee-conversion/checkUpkeep/checkUpkeep.t.sol:32
32: function test_WhenMarketsHaveMoreThanMinFeeForDistribution( // @audit POC
33: )
34: external
35: givenCheckUpkeepIsCalled
36: {
37: uint256 marketId=3967010399546;
38: uint256 amount=115792089237316195423570985008687907853269984665640564039457584007913129639932;
39: uint256 minFeeDistributionValueUsd=9306648518645823426120026427;
40:
41: PerpMarketCreditConfig memory fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
42:
43: minFeeDistributionValueUsd = bound({
44: x: minFeeDistributionValueUsd,
45: min: 1,
46: max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
47: });
48:
49: amount = bound({
50: x: amount,
51: min: minFeeDistributionValueUsd,
52: max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
53: });
54: deal({ token: address(usdc), to: address(fuzzPerpMarketCreditConfig.engine), give: amount });
55:
56: changePrank({ msgSender: users.owner.account });
57:
58: configureFeeConversionKeeper(1, 1);
59:
60: changePrank({ msgSender: address(fuzzPerpMarketCreditConfig.engine) });
61:
62: marketMakingEngine.receiveMarketFee(fuzzPerpMarketCreditConfig.marketId, address(usdc), amount);
63:
64: (bool upkeepNeeded, bytes memory performData) = FeeConversionKeeper(feeConversionKeeper).checkUpkeep("");
65:
66: (uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
67: console.log("marketIds" , marketIds.length);
68: // it should return true
69: assertTrue(upkeepNeeded);
70:
71: assertEq(assets[0], address(usdc));
72: assertEq(marketIds[0], fuzzPerpMarketCreditConfig.marketId);
73:
74: FeeConversionKeeper(feeConversionKeeper).performUpkeep(performData);
75: }

Output

​
│ │ │ │ ├─ [0] 0x0000000000000000000000000000000000000000::totalAssets() [staticcall]
│ │ │ │ │ └─ ← [Stop]
│ │ │ │ └─ ← [Revert] EvmError: Revert
​

The revert occurs when accessing an uninitialized zero address index, causing totalAssets() to fail.

Impact

performUpkeep will always revert, preventing fee distribution.

Tools Used

Manual Review

Recommendations

Dynamically allocate arrays based on actual asset count instead of using an arbitrary multiplier.

Updates

Lead Judging Commences

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