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

The claimAndSwap function fails to validate that the _routeNumber parameter is less than nRoutes

aariiif

Summary

In StrategyMainnet.sol where the claimAndSwap() function allows accessing invalid array indices in the route mappings.

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
// @audit-issue Missing bounds check on _routeNumber allows accessing invalid route configurations
// When _routeNumber >= nRoutes, this accesses uninitialized mapping values
transmuter.claim(_amountClaim, address(this));
router.exchange(
routes[_routeNumber], // @audit-issue Accessing invalid route data
swapParams[_routeNumber], // @audit-issue Accessing invalid swap params
_amountClaim,
_minOut,
pools[_routeNumber], // @audit-issue Accessing invalid pool data
address(this)
);

  1. The nRoutes variable tracks the number of valid route configurations

  2. The routes, swapParams, and pools mappings store route configurations indexed from 0 to nRoutes-1

  3. The claimAndSwap() function accepts any uint256 value for _routeNumber without validation

  4. When _routeNumber >= nRoutes, the function accesses uninitialized mapping values

  5. This violates the invariant proven by Certora that _routeNumber must be less than nRoutes

The vulnerability exists because the code assumes route numbers will be valid without explicit validation, allowing access to uninitialized or invalid route configurations that could cause swaps to fail after tokens have been claimed from the transmuter.

Vulnerability Details

The bug is an array bounds violation in StrategyMainnet.sol that allows accessing invalid route configurations.

// State variables storing route configurations
uint256 public nRoutes = 0;
mapping(uint256 => address[11]) public routes;
mapping(uint256 => uint256[5][5]) public swapParams;
mapping(uint256 => address[5]) public pools;
​
function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber // @audit-issue Unbounded uint256 input
) external onlyKeepers {
// @audit-issue Claims tokens before validating route exists
transmuter.claim(_amountClaim, address(this));
​
// @audit-issue Accesses potentially invalid route data when _routeNumber >= nRoutes
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
}

  1. Routes are added via addRoute() which increments nRoutes

  2. claimAndSwap() accepts any uint256 as _routeNumber without bounds checking

  3. When _routeNumber >= nRoutes, uninitialized mapping values are passed to the router

  4. The claim operation happens before route validation, potentially stranding claimed tokens

Impact

Flow:

  1. Keeper calls claimAndSwap with invalid route number

  2. Transmuter.claim() succeeds, transferring tokens to strategy

  3. Router.exchange() receives uninitialized route data

  4. Exchange fails, leaving claimed tokens stuck in contract

  5. No way to recover tokens without management intervention

The vulnerability breaks the assumption that route configurations are valid when performing swaps. This creates a risk of locked funds when invalid routes are used.

Recommendations

mitigation for the route number validation issue in StrategyMainnet.sol

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
+ // @audit-fix Validate route number before any token operations
+ require(_routeNumber < nRoutes, "Invalid route number");
+
+ // @audit-fix Validate route configuration exists
+ 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));
require(_minOut > _amountClaim, "minOut too low");
​
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

Additional recommendations:

  1. Add route validation in addRoute()

function addRoute(
address[11] calldata _route,
uint256[5][5] calldata _swapParams,
address[5] calldata _pools
) external onlyManagement {
+ // @audit-fix Validate route parameters
+ require(_route[0] != address(0), "Invalid route start");
+ require(_pools[0] != address(0), "Invalid pool address");
​
routes[nRoutes] = _route;
swapParams[nRoutes] = _swapParams;
pools[nRoutes] = _pools;
nRoutes++;
}

Add route removal capability

+// @audit-fix Allow management to remove invalid routes
+function removeRoute(uint256 _routeNumber) external onlyManagement {
+ require(_routeNumber < nRoutes, "Invalid route number");
+
+ delete routes[_routeNumber];
+ delete swapParams[_routeNumber];
+ delete pools[_routeNumber];
+
+ // Shift remaining routes down if not last route
+ if (_routeNumber != nRoutes - 1) {
+ for(uint i = _routeNumber; i < nRoutes - 1; i++) {
+ routes[i] = routes[i + 1];
+ swapParams[i] = swapParams[i + 1];
+ pools[i] = pools[i + 1];
+ }
+ }
+ nRoutes--;
+}
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.