Alchemix Transmuter
Alchemix
DeFiFoundrySolidity
16,653 OP
View results
Previous Next
Submission Details
Severity: low
Valid

Previous router approvals are not revoked during upgrades

gioruggieri

Summary

When updating the router through the setRouter function, approvals to the old router are not revoked. This means that previous routers maintain their authorization to spend the contract's tokens even after being replaced.

Vulnerability Details

The setRouter function in StrategyOp.sol updates the router address and approves the new router without revoking the approval from the previous router:

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

Impact

If a previous router becomes compromised or has vulnerabilities, it maintains the authorization to spend an unlimited amount of underlying tokens (WETH) from the contract, even after being replaced. This could lead to the loss of all contract funds. The vulnerability is present in all three contracts (StrategyOp.sol, StrategyMainnet.sol, and StrategyArb.sol)

It is particularly critical given the use of infinite approvals (type(uint256).max)

Tools Used

Manual code review

Static analysis

Proof of Concept

Contract is deployed with Router A which receives approval for type(uint256).max

Management upgrades to Router B via setRouter

Router B receives approval for type(uint256).max

Router A still maintains approval for type(uint256).max

If Router A is compromised, it can spend all underlying tokens of the contract

*// 1. Initial deployment*
​
strategy = new StrategyOp(asset, transmuter, "Strategy V1");
​
address routerA = strategy.router();
​
*// 2. Upgrade to new router*
​
address routerB = address(new MockRouter());
​
vm.prank(management);
​
strategy.setRouter(routerB);
​
*// 3. Verify routerA still has approval*
​
assertEq(
​
underlying.allowance(address(strategy), routerA),
​
type(uint256).max
​
);
​
*// 4. Compromised routerA can still spend tokens*
​
vm.prank(routerA);
​
underlying.transferFrom(
​
address(strategy),
​
attacker,
​
underlying.balanceOf(address(strategy))
​
);
​

Recommendations

Implement approval revocation as shown above

Add events to track router changes

function setRouter(address \_router) external onlyManagement {
​
require(\_router != address(0), "Zero address");
​
​
*// Revoke approval from old router*
​
if(router != address(0)) {
​
underlying.safeApprove(router, 0);
​
}
​
​
address oldRouter = router;
​
router = \_router;
​
​
*// Approve new router*
​
underlying.safeApprove(router, type(uint256).max);
​
​
emit RouterChanged(oldRouter, \_router);
​
}
​
Updates

Appeal created

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

Old router approval is not revoked after an update

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