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

Missing Validation for `_routeNumber` in `claimAndSwap` Function

Fortis Audits

Summary

The claimAndSwap function lacks a validation check to ensure that the _routeNumber parameter is within bounds of the existing routes (nRoutes). If an invalid _routeNumber is passed by mistake of keeper , it could result in accessing uninitialized data (e.g., address(0) for routes[_routeNumber], swapParams[_routeNumber], and pools[_routeNumber]), potentially causing the router call to revert or fail unexpectedly.

Vulnerability Details

The issue arises in the claimAndSwap function, where _routeNumber is directly used to access mappings (routes, swapParams, and pools) without bounds checking:

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

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
// @audit-issue : Missing check for _routeNumber < nRoutes.
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));
}

Explanation:

  • The _routeNumber parameter specifies which route to use for the swap.

  • If _routeNumber exceeds the value of nRoutes (the number of valid routes), the mappings return uninitialized values (e.g., address(0)).

  • This can lead to the following issues:

    • A call to router.exchange may revert or fail unexpectedly.

    • Potential unintended behavior or vulnerabilities if uninitialized data is used in other logic.

Impact

Functional Impact:

  • The claimAndSwap function may revert or fail unexpectedly when called with an invalid _routeNumber.

  • Operational inefficiency due to potential disruptions in token swapping functionality.

  • Security Risk:

    • Although unlikely, if the router does not revert, this could open up avenues for unintended behavior.

Tools Used

Manual review

Recommendations

Add a validation check at the beginning of the claimAndSwap function to ensure that _routeNumber is within bounds.

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
++ require(_routeNumber < nRoutes, "Invalid route number");
​
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));
}
Updates

Appeal created

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