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

If there is loss anyone can withdraw their deposited tokens without incurring any loss if it is within the strategies withdraw limit

aamirusmani1552

Summary

If we have some loss in the strategy, that loss will not be taken into account if user withdraws tokens from the strategy and have available withdraw limit for that.

Vulnerability Details

In the strategy, only way a profit/loss is realized is when report(...) is called. And that is not taken into account when a user withdraws those tokens. There will only be losses if user are withdrawing tokens that are not within the withdraw limit of the strategy. During withdraw, the strategy will call the _maxWithdraw(...) first to get the available withdraw limit:

function withdraw(
uint256 assets,
address receiver,
address owner,
uint256 maxLoss
) public nonReentrant returns (uint256 shares) {
// Get the storage slot for all following calls.
StrategyData storage S = _strategyStorage();
require(
@> assets <= _maxWithdraw(S, owner),
"ERC4626: withdraw more than max"
);
}

_maxWithdraw() calls availableWithdrawLimit(...) function and this is how the available limit is calculated:

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));
}

so it's just the total alETH balance of the transmuter and strategy iteself.

Then freeFunds() method is called to withdraw the required tokens from the transmuter if not have already:

function _withdraw(
StrategyData storage S,
address receiver,
address owner,
uint256 assets,
uint256 shares,
uint256 maxLoss
) internal returns (uint256) {
​
if (idle < assets) {
// Tell Strategy to free what we need.
unchecked {
@> IBaseStrategy(address(this)).freeFunds(assets - idle);
}

_freeFunds(...):

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));
}
}

Github Link

So as far as the transmuter has enough balance to withdraw, not upcoming loss or profit will be realized. So if there is an upcoming loss, a user can just frontrun the transaction and withdraw his tokens before that so that no loss is incurred to him. This is the problem because when there is a profit, a user frontrunning the transaction will not have much impact as the profit is unlocked over some time not at the same moment. But in case of loss, the shares are burnt immediately:

function report()
external
nonReentrant
onlyKeepers
returns (uint256 profit, uint256 loss)
{
else {
// Expect we have a loss.
unchecked {
loss = oldTotalAssets - newTotalAssets;
}
​
// Check in case `else` was due to being equal.
if (loss != 0) {
// We will try and burn the unlocked shares and as much from any
// pending profit still unlocking to offset the loss to prevent any PPS decline post report.
sharesToBurn = Math.min(
// Cannot burn more than we have.
S.balances[address(this)],
// Try and burn both the shares already unlocked and the amount for the loss.
@> _convertToShares(S, loss, Math.Rounding.Down) + sharesToBurn
);
}
​
// Check if there is anything to burn.
if (sharesToBurn != 0) {
@> _burn(S, address(this), sharesToBurn);
}
}

Impact

A user can get away without paying his losses share.

Tools Used

  • Foundry

  • Manual Review

Recommendations

It is recommended to have some kind of locking mechanism before report is called or go for some other alternative.

Updates

Lead Judging Commences

inallhonesty Lead Judge
8 months ago

Appeal created

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
aamirusmani1552 Submitter
8 months ago
aamirusmani1552 Submitter
8 months ago
aamirusmani1552 Submitter
8 months ago
inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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