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

Infinite Token Approvals Not Revoked When Changing Router

Summary

A critical vulnerability has been identified in the StrategyArb contract's setRouter function, which allows old routers to retain infinite approval over the strategy's WETH tokens, even after being replaced.

Disclaimer: This vulnerability requires the attacker to have previously had the role of router approved by MANAGEMENT. Although this limits the attack vector, the impact remains critical, as a compromised former router can steal all funds.

Vulnerability Details

In StrategyArb.sol, the setRouter function gives infinite approval to the new router without revoking that of the old one:

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max); // Donne une nouvelle approbation sans révoquer l'ancienne
}

PoC demonstrating operation:

function testApprovalAttack() public {
// Initial balance check
assertEq(ERC20(WETH).balanceOf(address(strategy)), 100 ether, "Initial balance wrong");
// Management changes to new router
vm.prank(MANAGEMENT);
strategy.setRouter(newRouter);
// Old router still has approval and can drain funds
oldRouter.drainAllowance(WETH, address(strategy));
// Verify the attack worked - funds were stolen by the old router
assertEq(ERC20(WETH).balanceOf(ATTACKER), 100 ether, "Funds not stolen");
}

Impact

  • Severity: High

  • An old malicious or compromised router may continue to move the strategy's WETH funds even after being replaced.

  • Potential loss of all strategy WETH funds

  • Direct and significant financial impact

Recommendations

  1. Revoke approval of the old router before approving a new one:

function setRouter(address _router) external onlyManagement {
// First revoke approval of the old router
if(router != address(0)) {
underlying.safeApprove(router, 0);
}
router = _router;
underlying.safeApprove(router, type(uint256).max);
}
  1. Use a more secure approval pattern:

  • Avoid infinite approvals

  • Approve only the amount needed for each transaction

  • Use increaseAllowance/decreaseAllowance rather than approve when possible

  1. Add additional controls:

  • Check that the new router is different from the old one

  • Implement a whitelist of authorized routers

  • Add delays between router changes

Updates

Appeal created

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

Old router approval is not revoked after an update

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