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

Approval Mismanagement Post-Router Update

Summary

The StrategyOp contract has a critical over sight where token approvals for the old router are not revoked when a new router is set using the setRouter function. The oversight allows the old router to retain unlimited access to the strategy’s tokens (underlying), even after a new router is configured. If the old router is compromised or malicious, this can lead to unauthorized transfers or fund drains. Which can be common case by using couple routers by time.

revoking approvals for the previous router is a must before setting a new one and adopting safer approval patterns to minimize exposure.

Technical Details

1. Code Reference

The setRouter function updates the router address and grants unlimited approval to the new router but does not revoke approvals for the old router:

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyOp.sol#L48

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

2. Workflow Context

  • During initialization, the contract approves unlimited tokens (underlying) to the router for swaps:

    function _initStrategy() internal {
    router = 0xa062aE8A9c5e11aaA026fc2670B0D65cCc8B2858;
    underlying.safeApprove(address(router), type(uint256).max);
    }
  • If the setRouter function is later called, a new router is approved without resetting or revoking the approval for the old router.

This leaves both the old and new routers with unlimited access to the strategy’s tokens.

Exploitation Scenarios

Scenario 1: Router Replacement Exploit

  1. Initial Setup: The router is set to Router A with unlimited approval.

  2. Router Update: The Manager updates the router to Router B using setRouter. Router A still retains approval.

  3. Attack: A malicious actor exploits Router A to drain the strategy’s tokens:

    function maliciousTransfer() external {
    underlying.transfer(attackerWallet, underlying.balanceOf(strategyContract));
    }
  4. Result: The attacker drains all tokens (underlying) via the old router, resulting in a complete asset loss.


Scenario 2: Unintended Token Transfers

  1. Both Router A (old) and Router B (new) have unlimited approvals.

  2. Unmonitored or accidental function calls to Router A result in unauthorized swaps or transfers.

  3. Result: Tokens are unintentionally misused, reducing user funds and disrupting operations.


Proof of Concept (PoC)

Reproduction:

  1. Deploy the contract and initialize the router using _initStrategy.

  2. Update the router using setRouter but without revoking the old router’s approval.

  3. Simulate an attacker exploiting the old router to transfer tokens:

    function maliciousTransfer() external {
    underlying.transfer(attackerWallet, underlying.balanceOf(strategyContract));
    }
  4. Observe the unauthorized transfer of tokens via the old router.

Severity: High

  1. Financial Impact:

    • The old router’s access to tokens enables complete fund drains or misuse.

    • Example: If the strategy holds 15,000 WETH (~$30M at $2,000/WETH), a compromised router can transfer the entire balance.

  2. Reputational Impact:

    • Users lose trust in the protocol due to mismanagement of token approvals.

  3. Operational Impact:

    • Fund drains disrupt strategy operations, requiring immediate intervention.

Root Cause Analysis

  1. Approval Mismanagement:

    • The setRouter function updates the router address but does not revoke approvals for the previous router.

  2. Unrestricted Approvals:

    • Approvals are set to type(uint256).max without constraints, allowing unlimited token transfers.

Mitigation Recommendations

1. Revoke Old Approvals

  • Revoke approvals for the old router before granting approval to the new router:

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

2. Limit Approvals

  • Approve only the necessary token amount for specific transactions:

    function approveAmount(address spender, uint256 amount) internal {
    underlying.safeApprove(spender, amount);
    }
Updates

Appeal created

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

Old router approval is not revoked after an update

inallhonesty Lead Judge 10 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.