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

Missing Swap Path Validation Enables Token Route Manipulation

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:

  1. Swaps through unintended or manipulated paths

  2. Use of malicious intermediary tokens

  3. Excessive slippage through unnecessarily long paths

  4. Trades that start/end with wrong tokens

  5. 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"); // Reasonable maximum path length
// Validate start and end tokens
require(_path[0].from == address(underlying), "Start token must be underlying");
require(_path[_path.length-1].to == address(asset), "End token must be asset");
// Validate path connections
for(uint i = 0; i < _path.length; i++) {
// Check valid addresses
require(_path[i].from != address(0) && _path[i].to != address(0), "Invalid route address");
// Check path continuity
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

  1. Consider whitelisting allowed intermediary tokens

  2. Add maximum path length configuration

  3. Implement slippage control per path segment

  4. Add events for path changes and validation failures

References

Test Cases

function testSwapPathValidation() public {
// Setup invalid paths
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));
// Test invalid start token
vm.prank(keeper);
vm.expectRevert("Start token must be underlying");
strategy.claimAndSwap(1e18, 1e18, invalidStartToken);
// Test disconnected path
vm.prank(keeper);
vm.expectRevert("Disconnected path");
strategy.claimAndSwap(1e18, 1e18, disconnectedPath);
}

Mitigation Status

Pending implementation

Proof of Concept

Prerequisites

  • Keeper role access

  • Strategy with claimable tokens

  • DEX with multiple possible paths

Steps to Reproduce

  1. Deploy strategy

  2. Setup invalid swap path with:

    • Wrong start token

    • Wrong end token

    • Disconnected intermediary tokens

  3. Call claimAndSwap with invalid path

  4. Observe successful transaction with unintended routing

  5. Verify token loss from poor route execution

Updates

Lead Judging Commences

inallhonesty Lead Judge
5 months ago

Appeal created

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 5 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.