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

Infinite Approval Not Reset When Changing the Router

Summary

The StrategyArb contract fails to reset the infinite approval of the underlying token when updating the router address. This oversight allows the previous router to retain unlimited access to the underlying tokens, posing a significant risk of unauthorized token transfers and potential loss of funds if the old router is compromised or malicious.

Vulnerability Details

In the contract, the router address can be updated by the management through the setRouter function:

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

When setRouter is called, the contract:

  1. Updates the router state variable to the new _router address.

  2. Grants unlimited approval (type(uint256).max) to the new router for spending the underlying tokens.

However, it does not reset the approval of the old router, meaning the previous router retains its infinite approval. If the old router is malicious or becomes compromised, it can transfer all the underlying tokens from the strategy contract at any time.

This vulnerability arises because ERC20 token approvals are not automatically revoked when a spender is no longer needed. Without explicitly setting the allowance to zero for the old router, it continues to have the same permissions as before.

Impact

  • Loss of Funds: A compromised or malicious old router can transfer all underlying tokens from the strategy contract, leading to a complete loss of those funds.

  • Security Risk: Unauthorized access to the strategy's assets undermines the security and trust of the system.

  • Compliance Issues: Failure to properly manage token allowances may violate best practices and regulatory requirements for asset management.

Proof of Concept (POC)

  1. Initial Setup:

    • The strategy contract has RouterA set as the current router.

    • RouterA has infinite approval to transfer underlying tokens on behalf of the strategy.

  2. Router Update:

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

    • The infinite approval is granted to RouterB, but no approval is revoked from RouterA.

  3. Exploit Execution:

    • An attacker gains control of RouterA (e.g., through a security breach or if RouterA was malicious from the start).

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

// Attacker's code within RouterA
function exploit(address token, address strategy) external {
uint256 amount = ERC20(token).balanceOf(strategy);
ERC20(token).transferFrom(strategy, msg.sender, amount);
}
  1. Result:

    • The strategy contract loses all its underlying tokens.

    • The attacker now holds all the tokens that were meant to be managed securely by the strategy.

Recommendations

  • Reset Approval for Old Router:

    • Before updating the router address, explicitly set the allowance of the old router to zero.

    • This ensures that only the new router has permission to transfer underlying tokens.

function setRouter(address _router) external onlyManagement {
// Reset approval for the old router
underlying.safeApprove(router, 0);
// Update the router
router = _router;
// Set approval for 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

Support

FAQs

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