Summary
The strategy contracts lack proper validation of swap paths in their router integration, allowing potential manipulation of token routes during swaps. This could lead to trades executing through unexpected or malicious paths, potentially resulting in significant losses through poor pricing or malicious routing.
Vulnerability Details
The swap functions in all strategy variants accept routing paths without validating:
Start/end token addresses match expected underlying/asset
Path continuity (each hop connects properly)
Valid intermediary token addresses
Reasonable path length
Code Snippets
From StrategyOp.sol:
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
From StrategyArb.sol and StrategyMainnet.sol (similar pattern):
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path) internal {
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IRamsesRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
Impact
Severity: High
The missing validation could allow:
Swaps through unintended or manipulated paths
Use of malicious intermediary tokens
Excessive slippage through unnecessarily long paths
Trades that start/end with wrong tokens
Failed transactions due to invalid path connections
Even with keepers being trusted, mistakes in path configuration could lead to permanent loss of funds.
Tools Used
Manual code review
Recommendations
Implement comprehensive path validation:
function _validatePath(IVeloRouter.route[] calldata _path) internal view {
require(_path.length > 0, "Empty path");
require(_path.length <= 4, "Path too long");
require(_path[0].from == address(underlying), "Start token must be underlying");
require(_path[_path.length-1].to == address(asset), "End token must be asset");
for(uint i = 0; i < _path.length; i++) {
require(_path[i].from != address(0) && _path[i].to != address(0), "Invalid route address");
if(i > 0) {
require(_path[i].from == _path[i-1].to, "Disconnected path");
}
}
}
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
_validatePath(_path);
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}
Additional Recommendations
Consider whitelisting allowed intermediary tokens
Add maximum path length configuration
Implement slippage control per path segment
Add events for path changes and validation failures
References
Test Cases
function testSwapPathValidation() public {
IVeloRouter.route[] memory invalidStartToken = new IVeloRouter.route[]();
invalidStartToken[0] = IVeloRouter.route(address(0x123), address(asset), true, address(0));
IVeloRouter.route[] memory disconnectedPath = new IVeloRouter.route[]();
disconnectedPath[0] = IVeloRouter.route(address(underlying), address(0x123), true, address(0));
disconnectedPath[1] = IVeloRouter.route(address(0x456), address(asset), true, address(0));
vm.prank(keeper);
vm.expectRevert("Start token must be underlying");
strategy.claimAndSwap(1e18, 1e18, invalidStartToken);
vm.prank(keeper);
vm.expectRevert("Disconnected path");
strategy.claimAndSwap(1e18, 1e18, disconnectedPath);
}
Mitigation Status
Pending implementation
Proof of Concept
Prerequisites
Steps to Reproduce
Deploy strategy
Setup invalid swap path with:
Call claimAndSwap with invalid path
Observe successful transaction with unintended routing
Verify token loss from poor route execution