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

Underlying is always accounted but never collected, leading to incorrect profit calculation

Dowsers

Description

Alchemix strategies use the underlying state to keep track of the WETH address. This is used several times in the code to determine the WETH balance of the strategies. Only strategies have access to that variable (inherited contracts never use it). The problem is that the WETH balance is used in balanceDeployed() and especially in _harvestAndReport() to calculate profit and loss. As explained by the code, it is to account for possible some dormant WETH that isn't swapped yet. However, there is absolutely no function to swap/collect them. So every WETH sent to any strategy will be blocked in the contract and corrupt the profit and loss calculation in BaseStrategy::report().

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
@> // @audit Should take the remaining underlying tokens, swap them and deposit them in the transmuter
}
​
...
​
function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
​
uint256 claimable = transmuter.getClaimableBalance(address(this));
​
if (claimable > 0) {
// transmuter.claim(claimable, address(this));
}
​
// NOTE : we can do this in harvest or can do seperately in tend
// if (underlying.balanceOf(address(this)) > 0) {
// _swapUnderlyingToAsset(underlying.balanceOf(address(this)));
// }
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
​
// NOTE : possible some dormant WETH that isn't swapped yet
@> uint256 underlyingBalance = underlying.balanceOf(address(this));
​
@> _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
​
...
​
function balanceDeployed() public view returns (uint256) {
@> return transmuter.getUnexchangedBalance(address(this)) + underlying.balanceOf(address(this)) + asset.balanceOf(address(this));
}

Risk

Likelyhood: High

  • Any person can transfer WETH to the contract at any time.

  • The 3 strategies are concerned.

Impact: High

  • _harvestAndReport() is used to calculate the profit/loss, and that calculation will always be wrong since those tokens are not real profit, they are just stuck.
    The returned value of this function, as explained in the natspec, has to be trusted and accurate because it will increase the collected fees and the newProfitLockingPeriod !

  • balanceDeployed() will return a incorrect value.

Recommended Mitigation

Several solutions are possible:

  • Uncomment the lines in _harvestAndReport() and patch them to use the correct _swapUnderlyingToAsset function (with the missing parameters).

  • claimAndSwap could include those lines to collect the remaining amount just before depositing the alETH balance:

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
+ uint underlyingBalance = underlying.balanceOf(address(this));
+ // minOut == underlyingBalance because we want to swap at premium or at least 1:1
+ _swapUnderlyingToAsset(underlyingBalance, underlyingBalance, _path);
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
Updates

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH in the contract will never be swapped back to alETH

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH is not properly treated

Support

FAQs

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