Part 2

Summary

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/main/src/external/chainlink/keepers/fee-conversion-keeper/FeeConversionKeeper

The function marketMakingEngine.convertAccumulatedFeesToWeth() is called inside a loop.

When convertAccumulatedFeesToWeth() transfers ETH or interacts with an external contract, an attacker could exploit this by reentering the function before the state is updated. This could lead to unexpected multiple executions, draining funds or causing incorrect accounting.

IMPACT:

A malicious contract could trigger reentrancy by calling performUpkeep() repeatedly, causing:

Double execution of fee conversion

Potential loss of funds if ETH is withdrawn multiple times

Disruption of market-making operations

Fix:

Use ReentrancyGuard and apply the nonReentrant modifier to performUpkeep().

Follow the Checks-Effects-Interactions pattern (update state before calling external contracts).

Tools Used

Manual Review

Recommendations

Modify the function as follows:

function performUpkeep(bytes calldata performData) external override onlyForwarder nonReentrant {
updateState(); // Update contract state first
marketMakingEngine.convertAccumulatedFeesToWeth(); // External call AFTER state update
}

This ensures state changes are committed before external interactions, preventing reentrancy attacks.