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

Reentrancy in performUpkeep

mibrobuba

Summary


The performUpkeep function calls marketMakingEngine.convertAccumulatedFeesToWeth, which may involve external calls (e.g., to a DEX adapter). If the external contract is malicious or compromised, it could re-enter the performUpkeep function, potentially leading to unexpected behavior or loss of funds.

Vulnerability Details

function https://github.com/Cyfrin/2025-01-zaros-part-2/blob/35deb3e92b2a32cd304bf61d27e6071ef36e446d/src/external/chainlink/keepers/fee-conversion-keeper/FeeConversionKeeper.sol#L110-L127performUpkeep(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, "");
}
}
​
​
/// @notice Retrieves the current configuration of the fee conversion keeper.
/// @return keeperOwner The address of the owner of the fee conversion keeper.
/// @return marketMakingEngine The address of the market-making engine.
/// @param minFeeDistributionValueUsd The minimum fee distribution value in USD.

Impact

The function calls marketMakingEngine.convertAccumulatedFeesToWeth, which may involve an external call to a DEX adapter or another contract.

  • If the external contract (e.g., DEX adapter) is malicious, it can call back into the performUpkeep function before the original call completes.

  • During reentrancy, the state of the contract (e.g., fee balances, market data) may be inconsistent, allowing the attacker to manipulate the system.

  • Reentrancy could allow an attacker to manipulate the state of the contract or drain funds.

Tools Used

Manual Code review

Recommendations

  • To prevent reentrancy, use OpenZeppelin's ReentrancyGuard and mark the performUpkeep function as nonReentrant

  • Adding a reentrancy guard (nonReentrant modifier) ensures that the function cannot be re-entered during execution, mitigating the vulnerability.\

import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract FeeConversionKeeper is IAutomationCompatible, BaseKeeper, ReentrancyGuard {
function performUpkeep(bytes calldata performData) external override nonReentrant onlyForwarder {
FeeConversionKeeperStorage memory self = _getFeeConversionKeeperStorage();
​
IMarketMakingEngine marketMakingEngine = self.marketMakingEngine;
​
// Decode performData
(uint128[] memory marketIds, address[] memory assets) = abi.decode(performData, (uint128[], address[]));
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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