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

Violation of EIP4626 Standard of `Strategy` Due to Improper Limit

jesjupyter

Summary

The StrategyArb, StrategyMainnet, and StrategyOp contracts inherit from BaseStrategy and are designed to integrate with the TokenizedStrategy to create fully ERC4626-compliant vaults. However, the strategies fail to implement the availableWithdrawLimit and availableDepositLimit correctly to have EIP4626-compliant maxMint, maxDeposit, maxWithdraw and maxRedeem. leading to incompatibility with EIP4626 standards. This issue can result in unexpected Denial-of-Service (DoS) for other users and contracts.

Vulnerability Details

According to the code comment and documentation, the entire vault should be fully ERC4626 compliant.

A Strategy contract can become a fully ERC-4626 compliant vault by inheriting the BaseStrategy contract, that uses the fallback function to delegateCall the previously deployed version of TokenizedStrategy.

A strategist only needs to override a few simple functions that are focused entirely on the strategy specific needs to easily and cheaply deploy their own permissionless 4626 compliant vault.

However, the strategy fails to correctly implement vault-related limits, for example availableWithdrawLimit, leading to incompatibility with EIP4626 standards.

TL&DR: The availableWithdrawLimit function only considers available funds (asset.balanceOf(address(this)) and unexchanged balances from the transmuter). It fails to account for conditions where the transmuter.withdraw function becomes unusable due to the strategy being unwhitelisted or the transmuter being disabled.

Detailed explanation:

Let's take the maxWithdraw function for example(this is also the case for maxDeposit, maxMint and maxRedeem).
In EIP4626, to be compliant.

the maxWithdraw MUST return the maximum amount of assets that could be transferred from owner through withdraw and not cause a revert, which MUST NOT be higher than the actual maximum that would be accepted

Let's take a dive into the maxWithdraw function.
In TokenizedStrategy, it's calling _maxWithdraw which will call IBaseStrategy(address(this)).availableWithdrawLimit.

function maxWithdraw(address owner) external view returns (uint256) {
return _maxWithdraw(_strategyStorage(), owner);
}
/// @dev Internal implementation of {maxWithdraw}.
function _maxWithdraw(
StrategyData storage S,
address owner
) internal view returns (uint256 maxWithdraw_) {
// Get the max the owner could withdraw currently.
@=> maxWithdraw_ = IBaseStrategy(address(this)).availableWithdrawLimit(
owner
);
​
// If there is no limit enforced.
if (maxWithdraw_ == type(uint256).max) {
// Saves a min check if there is no withdrawal limit.
maxWithdraw_ = _convertToAssets(
S,
_balanceOf(S, owner),
Math.Rounding.Down
);
} else {
maxWithdraw_ = Math.min(
_convertToAssets(S, _balanceOf(S, owner), Math.Rounding.Down),
maxWithdraw_
);
}
}

For example, in StrategyArb, the availableWithdrawLimit is defined as below:

function availableWithdrawLimit(
address /*_owner*/
) public view override returns (uint256) {
// NOTE: Withdraw limitations such as liquidity constraints should be accounted for HERE
// rather than _freeFunds in order to not count them as losses on withdraws.
​
// TODO: If desired implement withdraw limit logic and any needed state variables.
​
// EX:
// if(yieldSource.notShutdown()) {
// return asset.balanceOf(address(this)) + asset.balanceOf(yieldSource);
// }
// NOTE : claimable balance can only be included if we are actually allowing swaps to happen on withdrawals
//uint256 claimable = transmuter.getClaimableBalance(address(this));
return asset.balanceOf(address(this)) + transmuter.getUnexchangedBalance(address(this));
}

However, when we look at the vault's _withdraw function, when the active assets are not enough, the freeFunds is called, which will eventually call transmuter.withdraw.

if (idle < assets) {
// Tell Strategy to free what we need.
unchecked {
IBaseStrategy(address(this)).freeFunds(assets - idle);
}
function _freeFunds(uint256 _amount) internal override {
uint256 totalAvailabe = transmuter.getUnexchangedBalance(address(this));
if (_amount > totalAvailabe) {
transmuter.withdraw(totalAvailabe, address(this));
} else {
transmuter.withdraw(_amount, address(this));
}
}

But in the Transmuter::withdraw, the function requires the strategy to be whitelisted, as defined in the Whitelist contract. If the strategy is unwhitelisted, withdrawals fail, leading to a revert.

/// @inheritdoc ITransmuterV2
function withdraw(uint256 amount, address recipient) external override nonReentrant {
_onlyWhitelisted();
_updateAccount(
UpdateAccountParams({
owner: msg.sender,
unexchangedDelta: -SafeCast.toInt256(amount),
exchangedDelta: 0
})
);
TokenUtils.safeTransfer(syntheticToken, recipient, amount);
emit Withdraw(msg.sender, recipient, amount);
}

There are several cases when isWhitelisted return false according to whitelist.sol: either the transmuter is disabled or the strategy is later unwhitelisted by the admin.

/// @inheritdoc IWhitelist
function isWhitelisted(address account) external view override returns (bool) {
return disabled || addresses.contains(account);
}

However, this case is not considered by the availableWithdrawLimit: when isWhitelisted returns false, the vault can not withdraw ALETH from the transmuter, thus the entire transaction will revert.

Per EIP4626, the maxWithdraw function must return the maximum amount of assets that can be withdrawn without causing a revert. The current implementation does not meet this requirement, as it fails to consider cases where transmuter.withdraw is inaccessible. This violates the EIP4626 standard as we said before. When others are interacting with the TokenizedStrategy, they could suffer from unexpected DoS.

Impact

The implementation of Strategy violates the EIP4626 standard, and is not fully compatible as claimed. Users interacting with the strategy may experience unexpected DoS when withdrawal attempts fail due to unconsidered scenarios.

Tools Used

Manual Review, EIP4626 Documentation

Recommendations

It is recommended to take these cases into account for the strategy to be fully compatible.

Updates

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID]Violation of EIP4626 Standard of `Strategy` Due to Improper Limit

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID]Violation of EIP4626 Standard of `Strategy` Due to Improper Limit

jesjupyter Submitter
6 months ago
inallhonesty Lead Judge
6 months ago
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID]Violation of EIP4626 Standard of `Strategy` Due to Improper Limit

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.