Missing Reentrancy Guard in Critical Functions

Issue:

The claimAndSwap and _freeFunds functions interact with external contracts (transmuter, router) without implementing reentrancy protection. If these external calls allow reentrancy, they could be exploited to manipulate the contract's state and logic, leading to potential fund drains or operational disruption.

Impact:

1. Fund Drains:

   • Exploiting reentrant calls allows attackers to withdraw more funds than intended.

2. State Corruption:

   • Reentrancy can disrupt internal accounting, leading to mismanagement of funds.

3. Operational Halt:

   • Other contract functions relying on accurate state information may fail, affecting the protocol's operations and trustworthiness.

Evidence from Code:

claimAndSwap Function:

_freeFunds Function:

Attack Path PoC

1. Reentrancy via transmuter.claim:

   Assume an attacker deploys a malicious transmuter contract that allows reentrant calls during the claim function execution.

2. Attack Setup:

   The attacker calls claimAndSwap with their address as the recipient and malicious transmuter logic to reenter the claimAndSwap function before state variables like balBefore or balances are updated.

3. Implementation:

   • Deploy a malicious transmuter contract:

   pragma solidity ^0.8.18;

   ​

   contract MaliciousTransmuter {

   address public strategy;

   ​

   constructor(address _strategy) {

   strategy = _strategy;

   }

   ​

   function claim(uint256 _amount, address _to) external {

   // Trigger reentrancy attack

   StrategyOp(strategy).claimAndSwap(_amount, _amount, new IVeloRouter.route );

   }

   ​

   function getClaimableBalance(address _owner) external pure returns (uint256) {

   return type(uint256).max; // Return an extremely large value

   }

   }

   ​

4. Execution Flow:

   • The attacker sets the malicious transmuter contract in the strategy.

   • Calls claimAndSwap:

     strategy.claimAndSwap(1000, 900, path);

     ​

   • The malicious transmuter reenters claimAndSwap before the state updates, exploiting incomplete logic to drain funds or cause unexpected behavior.

5. Result:

   1. Funds can be drained or state variables corrupted due to unprotected reentrant calls.

Mitigation

Implement Reentrancy Guards:

Use OpenZeppelin's ReentrancyGuard to protect critical functions.

Use Checks-Effects-Interactions Pattern:

Update internal state variables before making external calls.