DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Invalid

routeNumber must be less than nRoutes

Summary

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber // <-- Missing bounds check allows accessing undefined routes
) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
require(_minOut > _amountClaim, "minOut too low");
router.exchange(
routes[_routeNumber], // <-- Accessing potentially undefined route data
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);

The contract maintains a counter nRoutes that tracks valid route configurations, but fails to validate that _routeNumber parameter is within bounds (< nRoutes) before accessing route data from the mappings.

This allows:

  1. Accessing undefined routes beyond the valid range

  2. Using uninitialized or garbage data in exchange calls

  3. Potential manipulation by keepers providing invalid route numbers

Because the contract assumes all route numbers passed to claimAndSwap are valid without validation. This breaks a core safety assumption that only configured routes should be usable for exchanges, the function can be called with route numbers >= nRoutes, violating the safety property that all used routes must be previously configured.

Vulnerability Details

In StrategyMainnet.claimAndSwap()

// Root cause: No validation that routeNumber < nRoutes before accessing route data
// This allows accessing uninitialized or invalid route configurations
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
router.exchange(
routes[_routeNumber], // Can access undefined route
swapParams[_routeNumber], // Can access undefined params
_amountClaim,
_minOut,
pools[_routeNumber], // Can access undefined pool
address(this)
);
}
/**
* ╔═══════════════════════════════════════════════════════════════════════════════╗
* ║ VULNERABILITY IMPACT ANALYSIS ║
* ╠══════════════════╦════════════════╦═══════════════════╦══════════════════════╣
* ║ Strategy ║ Router ║ Chain Impact ║ Route Issues ║
* ╠══════════════════╬════════════════╬═══════════════════╬══════════════════════╣
* ║ StrategyMainnet ║ Curve Router ║ Failed swaps & ║ Complex route ║
* ║ ║ ║ unexpected routes ║ configurations ║
* ╠══════════════════╬════════════════╬═══════════════════╬══════════════════════╣
* ║ StrategyOp ║ Velodrome ║ Incorrect pool ║ Route arrays ║
* ║ ║ Router ║ access ║ validation ║
* ╠══════════════════╬════════════════╬═══════════════════╬══════════════════════╣
* ║ StrategyArb ║ Ramses Router ║ Malformed route ║ Route structures ║
* ║ ║ ║ data ║ handling ║
* ╠══════════════════╩════════════════╩═══════════════════╩══════════════════════╣
* ║ ROOT CAUSES ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ • Missing bounds validation for route data access ║
* ║ • Implicit trust in keeper-provided route numbers ║
* ║ • Lack of safety checks for uninitialized routes ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ TOKEN IMPACT ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ • WETH->alETH: Swap failures or incorrect routing ║
* ║ • Transmuter claims: Asset stuck risk ║
* ║ • Strategy shares: Unfavorable rate execution ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ KEEPER EXPLOITATION ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ • Can access undefined routes ║
* ║ • Can use uninitialized parameters ║
* ║ • Can route through non-existent pools ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ CRITICAL CONSIDERATIONS ║
* ╠════════════════════════════════════════════════════════════════════════════════
* ║ • Strategies handle user deposits ║
* ║ • Multiple protocol interactions (Alchemix, DEXes) ║
* ║ • Cross-chain operations with varied routing ║
* ╚════════════════════════════════════════════════════════════════════════════════
*/

Impact

StrategyMainnet

mapping(uint256 => address[11]) public routes;
mapping(uint256 => uint256[5][5]) public swapParams;
mapping(uint256 => address[5]) public pools;
// Routes are added by management
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++;
}
// But claimAndSwap lacks validation on routeNumber
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber // No validation against nRoutes
) external onlyKeepers {
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
}

Recommendations

// For StrategyMainnet.sol
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
+ // Validate route number is within bounds of configured routes
+ require(_routeNumber < nRoutes, "Invalid route number");
+
+ // Validate route configuration exists and is properly initialized
+ require(routes[_routeNumber][0] != address(0), "Route not configured");
+ require(pools[_routeNumber][0] != address(0), "Pool not configured");
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
+ // Additional validation for swap parameters
+ require(_amountClaim > 0, "Amount must be greater than 0");
require(_minOut > _amountClaim, "minOut too low");
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
}
+ // Add route validation helper
+ function validateRoute(uint256 _routeNumber) internal view returns (bool) {
+ return _routeNumber < nRoutes &&
+ routes[_routeNumber][0] != address(0) &&
+ pools[_routeNumber][0] != address(0);
+ }
// For StrategyOp.sol and StrategyArb.sol
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
IVeloRouter.route[] calldata _path
) external onlyKeepers {
+ // Validate path configuration
+ require(_path.length > 0, "Empty route path");
+ require(_path[0].from == address(underlying), "Invalid start token");
+ require(_path[_path.length-1].to == address(asset), "Invalid end token");
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
+ // Validate amounts and slippage
+ require(_amountClaim > 0, "Amount must be greater than 0");
require(_minOut > _amountClaim, "minOut too low");
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
}
Updates

Appeal created

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