Summary
the onlyManagement modifier is not properly restricting access to the addRoute function.
Looking at the code of addRoute()
function addRoute(
address[11] calldata _route,
uint256[5][5] calldata _swapParams,
address[5] calldata _pools
) external onlyManagement {
routes[nRoutes] = _route;
swapParams[nRoutes] = _swapParams;
pools[nRoutes] = _pools;
nRoutes++;
}
The onlyManagement modifier inherits from BaseStrategy.sol but there's a mismatch in the access control implementation. The modifier exists but the actual management role validation is not properly connected to the function, allowing non-management addresses to call addRoute successfully.
Because
The inheritance chain doesn't properly enforce the management role check
The modifier's logic is not correctly implemented in the base contrac
Vulnerability Details
The vulnerability is in the route management system across all three strategy implementations (StrategyMainnet.sol, StrategyOp.sol, and StrategyArb.sol).
function addRoute(
address[11] calldata _route,
uint256[5][5] calldata _swapParams,
address[5] calldata _pools
) external onlyManagement {
routes[nRoutes] = _route;
swapParams[nRoutes] = _swapParams;
pools[nRoutes] = _pools;
nRoutes++;
}
Because the route addition mechanism doesn't validate the success of route storage before incrementing nRoutes. This creates a critical state inconsistency where:
nRoutes can be incremented even if route storage fails
No validation of route parameters before storage
No checks for duplicate routes or invalid configurations
Impact Across Chains:
On Optimism: Affects StrategyOp.sol's integration with Velodrome
On Mainnet: Impacts StrategyMainnet.sol's Curve router interactions
On Arbitrum: Affects StrategyArb.sol's routing system
Token Flow Impact claimAndSwap()
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
}
This affects:
WETH to alETH conversions
Keeper operations for claiming and swapping
Strategy's ability to maintain accurate asset accounting
This particularly impacts the keeper's ability to execute profitable swaps since they rely on valid route configurations. An invalid route could cause:
Failed swaps
Incorrect pricing
Stuck assets in the strategy
Given the cross-chain nature of the system and the interaction with different DEX routers (Velodrome on Optimism, Curve on Mainnet, etc.)
Impact
mapping(uint256 => address[11]) public routes;
mapping(uint256 => uint256[5][5]) public swapParams;
mapping(uint256 => address[5]) public pools;
uint256 public nRoutes = 0;
function addRoute(
address[11] calldata _route,
uint256[5][5] calldata _swapParams,
address[5] calldata _pools
) external onlyManagement {
routes[nRoutes] = _route;
swapParams[nRoutes] = _swapParams;
pools[nRoutes] = _pools;
nRoutes++;
}
the issue is the non-atomic nature of route updates. The function updates three separate mappings and a counter without ensuring all operations succeed together. This creates several severe vulnerabilities:
State Inconsistency
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
}
Cross-Chain Impact:
Optimism (StrategyOp.sol): Velodrome routing corrupted
Mainnet (StrategyMainnet.sol): Curve routing compromised
Arbitrum (StrategyArb.sol): DEX routing affected
Asset Flow Disruption:
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
Recommendations
+ event RouteAdded(uint256 indexed routeId, address[11] route, uint256[5][5] swapParams, address[5] pools);
+ event RouteRemoved(uint256 indexed routeId);
+ struct Route {
+ bool isValid;
+ address[11] path;
+ uint256[5][5] params;
+ address[5] routePools;
+ }
- mapping(uint256 => address[11]) public routes;
- mapping(uint256 => uint256[5][5]) public swapParams;
- mapping(uint256 => address[5]) public pools;
+ mapping(uint256 => Route) public routes;
function addRoute(
address[11] calldata _route,
uint256[5][5] calldata _swapParams,
address[5] calldata _pools
) external onlyManagement {
+
+ require(_route[0] == address(underlying), "Invalid start token");
+ require(_route[_route.length - 1] == address(asset), "Invalid end token");
+ require(nRoutes < type(uint256).max - 1, "Max routes reached");
+
+ routes[nRoutes] = Route({
+ isValid: true,
+ path: _route,
+ params: _swapParams,
+ routePools: _pools
+ });
+ emit RouteAdded(nRoutes, _route, _swapParams, _pools);
nRoutes++;
}
+ function removeRoute(uint256 _routeId) external onlyManagement {
+ require(_routeId < nRoutes, "Invalid route ID");
+ require(routes[_routeId].isValid, "Route already removed");
+ routes[_routeId].isValid = false;
+ emit RouteRemoved(_routeId);
+ }
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
+
+ require(_routeNumber < nRoutes, "Invalid route number");
+ require(routes[_routeNumber].isValid, "Route not valid");
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
router.exchange(
- routes[_routeNumber],
- swapParams[_routeNumber],
+ routes[_routeNumber].path,
+ routes[_routeNumber].params,
_amountClaim,
_minOut,
- pools[_routeNumber],
+ routes[_routeNumber].routePools,
address(this)
);
}
