DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: low
Valid

Infinite Approval Not Reset When Changing the Router

Summary

The StrategyOp contract does not reset the infinite approval of the underlying token (WETH) when updating the router address through the setRouter function. This oversight allows the previous router to retain unlimited access to the underlying tokens. If the old router is compromised or malicious, it can transfer all the underlying tokens from the strategy contract, leading to significant financial loss.

Vulnerability Details

In the setRouter function, the contract updates the router address and grants it an infinite approval to spend underlying tokens:

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

Issue:

  • Approval Not Revoked from Old Router: When updating the router, the contract does not revoke the infinite approval granted to the previous router. As a result, the old router retains the ability to transfer unlimited underlying tokens from the strategy contract.

Impact

  • Loss of Funds: The old router can transfer all underlying tokens from the strategy, leading to complete loss of these assets.

  • Security Breach: Unauthorized access to the strategy's funds by a deprecated or malicious router compromises the contract's integrity.

  • Operational Risk: If the old router is compromised after being replaced, it can still exploit the infinite approval to drain funds.

Proof of Concept (POC)

  1. Initial Setup:

    • The strategy contract is set with RouterA as the router.

    • RouterA has infinite approval to spend underlying tokens (WETH) from the strategy contract.

  2. Router Update:

    • The management calls setRouter to update the router to RouterB.

    • The infinite approval is granted to RouterB.

    • Issue: No action is taken to revoke the approval from RouterA.

  3. Exploit Execution:

    • An attacker gains control over RouterA (e.g., through a security breach or because RouterA was malicious).

    • The attacker uses RouterA to transfer all underlying tokens from the strategy contract to an address under their control.

    // Inside the malicious RouterA contract
    function exploit(address token, address victim) external {
    uint256 balance = ERC20(token).balanceOf(victim);
    ERC20(token).transferFrom(victim, msg.sender, balance);
    }
  4. Result:

    • All underlying tokens are drained from the strategy contract.

    • Investors suffer a complete loss of the underlying assets managed by the strategy.

Recommendations

  • Revoke Approval from Old Router:

    • Before updating the router address, reset the approval for the previous router to zero.

    function setRouter(address _router) external onlyManagement {
    // Revoke approval from the old router
    underlying.safeApprove(router, 0);
    // Update to the new router
    router = _router;
    // Grant approval to the new router
    underlying.safeApprove(router, type(uint256).max);
    }
Updates

Appeal created

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

Support

FAQs

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