The addRoute
function in the StrategyMainnet
contract (src/StrategyMainnet.sol
) has several design limitations, including fixed array sizes, a lack of bounds checking, and no mechanism for removing or updating routes. These constraints restrict the contract's flexibility and scalability, particularly when the number of routes needs to exceed the fixed limit or when changes to existing routes are required.
Contract: StrategyMainnet.sol
Location: src/StrategyMainnet.sol:56-64
Interface: IStrategyInterface.sol
No Bounds Checking on nRoutes
:
In StrategyMainnet.sol
, there is no validation to prevent nRoutes
from exceeding the maximum size (5) of the arrays routes
, swapParams
, and pools
, risking an out-of-bounds error.
No Mechanism for Route Removal or Updates:
The contract does not support removing or updating routes in the mappings routes
, swapParams
, and pools
, which could be problematic if routes need to be changed or removed over time.
Fixed Array Sizes:
The contract uses fixed-size arrays:
address[11]
for routes
uint256[5][5]
for swap parameters
address[5]
for pools
This design limits the number of routes to 5 and each route to specific dimensions, which may not be scalable for future needs.
No Update Mechanism for Routes:
Once a route is added to the mappings, there is no functionality to modify it, forcing a full removal and re-insertion process which is currently not possible.
Medium/Low Risk:
The current limitations in StrategyMainnet.sol
do not pose immediate security risks but could impact the scalability and operational flexibility of the contract. If more than 5 routes are needed, or if routes require removal or updates, it may necessitate contract redeployment or significant changes to the protocol, which could disrupt operations.
No Direct Security Vulnerability:
While the vulnerability does not expose the contract to direct attacks, it introduces operational challenges in the Curve Router integration (ICurveRouterNG
) that could affect the contract's long-term viability.
Potential Operational Constraints:
As the Alchemix protocol evolves or scales, the lack of route management flexibility may cause operational issues, especially if the system needs more than 5 routes or dynamic updates to the routing configurations for the Curve Router integration.
Manual Code Review:
A thorough review of the addRoute
function and its associated logic reveals the design flaws.
Solidity Static Analysis:
Identifying array size limitations and bounds checking issues, which would be flagged in tools like Slither or MythX.
Gas Optimization Tools:
Considerations for optimizing the contract's functionality while maintaining flexibility (e.g., use of dynamic arrays).
Add Bounds Checking: Ensure that nRoutes
does not exceed the maximum number of routes (5) by adding a require
statement to prevent out-of-bounds writes in StrategyMainnet.sol
:
Implement Route Removal Functionality: Add a new function in StrategyMainnet.sol
to allow routes to be removed by shifting remaining routes down the arrays to fill the gap:
Consider Using Dynamic Arrays: Transition to dynamic arrays for better scalability. If this is a future consideration, ensure that appropriate safety checks (e.g., maximum route count) are implemented to prevent excessive gas usage.
Add Route Update Mechanism (Optional): Allow for updates to existing routes instead of requiring their removal and re-insertion. This would reduce gas costs and provide greater flexibility for managing routes.
By implementing these changes, the contract would gain flexibility, scalability, and operational robustness, ensuring it remains adaptable as the protocol evolves.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.