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

Router update must change router address

Summary

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max); // Here is the root cause - no validation on router address
}

The setRouter function allows setting the router to the same address as the current router. When this happens:

  1. The same router gets another max approval

  2. Unnecessary gas is spent

  3. The state transition effectively does nothing but still costs gas

The vulnerability is because the code assumes any router update is valid without checking if it actually changes the state. This is an incorrect assumption about state transitions.

Vulnerability Details

In StrategyArb.setRouter

function setRouter(address _router) external onlyManagement {
// Root cause: Router update doesn't revoke previous router's approval before setting new router
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

When updating the router across any of the strategies, the previous router's approval is never revoked. This means:

  1. Multiple routers maintain max approval to spend the strategy's underlying tokens (WETH)

  2. This accumulation of approvals increases attack surface

  3. If a previous router becomes compromised, it retains spending authority

Impact

In StrategyMainnet._initStrategy()

// Root cause: No approval revocation for previous router
function _initStrategy() internal {
router = ICurveRouterNG(0xF0d4c12A5768D806021F80a262B4d39d26C58b8D);
underlying.safeApprove(address(router), type(uint256).max);
}

In StrategyOp.setRouter

// Root cause: Unlimited approvals accumulate across router updates
function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

In StrategyArb.setRouter

// Root cause: Each router change adds new max approval without revoking old ones
function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

Recommendations

function setRouter(address _router) external onlyManagement {
require(_router != address(0), "Invalid router");
// Revoke old router approval
underlying.safeApprove(router, 0);
router = _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.