Summary
The FeeConversionKeeper.sol contract is a Chainlink Automation Contract that is Logic Based Triggered. The function checkUpkeep() returns (bool upkeepNeeded, bytes memory performData).
If upkeepNeeded is true, the exact returned value for performData will be used in the function performUpkeep(). Due to erroneous values for performData returned from checkUpKeep(), performUpkeep() will always revert.
Vulnerability Details
In the checkUpkeep() function, the performData is the prepared as such
performData = abi.encode(marketIds, assets);
However, marketIds and assets are first initialized as such:
uint128[] memory marketIds = new uint128[](liveMarketIds.length * 10);
address[] memory assets = new address[](liveMarketIds.length * 10);
Let's assume that liveMarketIds.length is 10, each array will have a size of 100, but only a small subset of this array may contain valid data.
The remaining slots are populated with invalid defaults of uint128(0) and address(0).
These invalid entries are encoded into performData and passed to performUpkeep(), which iterates over all slots:
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
for (uint256 i; i < marketIds.length; i++) {
marketMakingEngine.convertAccumulatedFeesToWeth(marketIds[i], assets[i], self.dexSwapStrategyId, "");
}
When the argument asset is address(0) , and is then passed into convertAccumulatedFeesToWeth(), collateral.verifyIsEnabled(); will cause the transaction to revert.
function convertAccumulatedFeesToWeth(
uint128 marketId,
address asset,
uint128 dexSwapStrategyId,
bytes calldata path
)
external
onlyRegisteredSystemKeepers
{
Collateral.Data storage collateral = Collateral.load(asset);
collateral.verifyIsEnabled();
Impact
Protocol Functionality affected:
Automation will always revert.
Fees accumulated in assets such as USDC cannot be converted to WETH, disrupting rewards distribution.
POC
pragma solidity 0.8.25;
import { Errors } from "@zaros/utils/Errors.sol";
import { Base_Test } from "test/Base.t.sol";
import { FeeConversionKeeper } from "@zaros/external/chainlink/keepers/fee-conversion-keeper/FeeConversionKeeper.sol";
contract FeeConversionKeeperPOC is Base_Test {
function setUp() public virtual override {
Base_Test.setUp();
changePrank({ msgSender: address(users.owner.account) });
createVaults(marketMakingEngine, INITIAL_VAULT_ID, FINAL_VAULT_ID, true, address(perpsEngine));
configureMarkets();
changePrank({ msgSender: address(users.naruto.account) });
}
modifier givenCheckUpkeepIsCalled() {
_;
}
function test_FeeConversionKeeperPerformData(
uint256 marketId,
uint256 amount,
uint256 minFeeDistributionValueUsd
)
external
givenCheckUpkeepIsCalled
{
PerpMarketCreditConfig memory fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
minFeeDistributionValueUsd = bound({
x: minFeeDistributionValueUsd,
min: 1,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
amount = bound({
x: amount,
min: minFeeDistributionValueUsd,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
deal({ token: address(usdc), to: address(fuzzPerpMarketCreditConfig.engine), give: amount });
changePrank({ msgSender: users.owner.account });
configureFeeConversionKeeper(1, 1);
changePrank({ msgSender: address(fuzzPerpMarketCreditConfig.engine) });
marketMakingEngine.receiveMarketFee(fuzzPerpMarketCreditConfig.marketId, address(usdc), amount);
(bool upkeepNeeded, bytes memory performData) = FeeConversionKeeper(feeConversionKeeper).checkUpkeep("");
assertTrue(upkeepNeeded);
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
assertEq(assets[0], address(usdc));
assertEq(marketIds[0], fuzzPerpMarketCreditConfig.marketId);
changePrank({ msgSender: users.owner.account });
FeeConversionKeeper(feeConversionKeeper).setForwarder(users.keepersForwarder.account);
changePrank({msgSender: users.keepersForwarder.account});
vm.expectRevert(abi.encodeWithSelector(Errors.CollateralDisabled.selector, address(0)));
FeeConversionKeeper(feeConversionKeeper).performUpkeep(performData);
}
}
Tools Used
Manual Review
Recommendations
Change the bottom part of the code to be
function checkUpkeep(){
if (upkeepNeeded) {
uint128[] memory correctMarketIds = new uint128[]();
address[] memory correctAssets = new address[]();
for (uint i; i< index; i++){
correctMarketIds[i] = marketIds[i];
correctAssets[i] = assets[i];
}
performData = abi.encode(correctMarketIds, correctAssets);
}
}
