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

Lack of input validation on `addRoute` can lead to DoS and loss of funds

Summary

The StrategyMainnet.sol contract allows adding new routes for swapping between the underlying asset and the strategy asset using Curve's router. However, there are two key issues:

  • No validation is performed on the data when adding a new route through the addRoute function. This could allow setting invalid or malicious routes.

  • The route data storage is not upgradeable. If an incorrect route is added, there is no way to update or remove it.

Vulnerability Details

Let's take a look on the addRoutefunction:

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L56-L65

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++;
}

See that no checks are performed on the _route, _swapParams or _pools data to ensure they represent a valid route between the expected underlying and strategy assets. This allows adding arbitrary routes. Besides, the routes, swapParams and pools state variables are mappings, with the route number as the key. Once a route is added, there is no way to update or remove that entry. The nRoutes counter only increments.

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L23-L25

mapping(uint256 => address[11]) public routes;
mapping(uint256 => uint256[5][5]) public swapParams;
mapping(uint256 => address[5]) public pools;

As a result, if an invalid route is added(i.e asset A is set to route B when it should be set to route A), or if a route is added with an incorrect key, there is no way to fix it. The incorrect route will remain indefinitely.

Impact

  • Invalid routes could be added that enable swapping the underlying asset for an unintended asset. This could lead to theft of funds if the keeper executes claimAndSwap with an incorrect route.

  • If an unintended route is added, even by mistake, there is no way to remove or update it. Over time this could lead to a large number of stale, invalid routes that pose risks but cannot be cleaned up.

  • DoS - swap cannot be executed in case of an incorrect/inexistent route.

Tools Used

Manual Review

Recommendations

Add checks in the addRoute function to validate that:

  • _route[0] is the address of the expected underlying asset

  • _route[_route.length-1] is the address of the expected strategy asset

  • The intermediary route hops represent a valid path between these

Also allow routes to be updated/removed.

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.