Alchemix Transmuter
Alchemix
DeFiFoundrySolidity
16,653 OP
View results
Previous Next
Submission Details
Severity: high
Invalid

When newRouter == routerBefore the update proceeds without reverting

aariiif

Summary

n the router update mechanism across all strategy contracts (StrategyArb.sol, StrategyOp.sol, StrategyMainnet.sol).

StrategyArb.setRouter

function setRouter(address _router) external onlyManagement {
// @audit-issue The function allows setting the router to the same address as current router
// This violates the specification which requires router updates to actually change the address
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

It is equired that when setRouter() is called, the router address must change to a new value different from the current one, but the implementation allows setting the router to the same address that's currently set.

The bug is because the implementation makes no comparison between the new and current router addresses before updating.

When newRouter == routerBefore:

  1. The update proceeds without reverting

  2. An unnecessary approval is granted

  3. The assertion router() != routerBefore fails

  4. Gas is wasted on a no-op transaction

Vulnerability Details

In StrategyArb.sol, StrategyOp.sol, and StrategyMainnet.sol:

function setRouter(address _router) external onlyManagement {
// @audit-issue State transition violation: Function allows setting router to same address
// This triggers unnecessary token approvals and violates the specification's
// requirement that router updates must change state
router = _router;
underlying.safeApprove(router, type(uint256).max);
}

The implementation violates a core state transition requirement - router updates must result in actual state changes. The code blindly updates and approves without validating if the new router differs from the current one.

Impact

Impact Flow:

  1. Management calls setRouter with current router address

  2. State remains unchanged but triggers token approval

  3. Underlying token gets unnecessarily re-approved for max amount

  4. Gas is wasted on redundant state updates

  5. Violates specification's state transition requirements

The bug affects all three strategy contracts since they share this implementation pattern:

  • StrategyArb.sol

  • StrategyOp.sol

  • StrategyMainnet.sol

Each strategy interacts with different DEX routers (Ramses, Velodrome, Curve) but shares the same flawed router update mechanism that allows redundant state updates and approvals.

Tools Used

Recommendations

// StrategyArb.sol, StrategyOp.sol, StrategyMainnet.sol
function setRouter(address _router) external onlyManagement {
+ // @audit-fix Validate router state transition - prevent redundant updates
+ require(_router != router, "Router: Cannot update to same address");
+
+ // @audit-fix Clear previous approval before updating router
+ underlying.safeApprove(router, 0);
+
router = _router;
underlying.safeApprove(router, type(uint256).max);
}
Updates

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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