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

not adding `claimable` balance to the total assets in `_harvestAndReport` can cause losses.

aamirusmani1552

Summary

The _harvestAndReport(...) does not taken into account the claimable transmuter.

Vulnerability Details

When report(...) is called on the strategy, it will call _harvestAndReport() that will get the new total assets by doing following things:

  1. Get the unclaimed balance of alETH deposited into the transmuter.

  2. Add the balance of any dormant WETH that is stored in the strategy itself.

  3. Add the alETH balance of the strategy.

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 (although we can restrict to only claim & swap in one tx)
uint256 underlyingBalance = underlying.balanceOf(address(this));
_totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}

Github Link

And all of the above combined will form the new total assets. And report function will check this with the old total assets. If it is more than the old we have a profit otherwise loss. And this is also when the new shares will be minted to the strategy and the total assets will be increased to match the new total assets. Over the time these new shares will be burned so that the profit is realized by the depositors. But in _harvestAndReport(...) function, claimable balance of WETH that is exchanged into the transmuter that is not taken into consideration probably because the claimAndSwap(...) is called before the report which will fetch the claimable balance and will exchange it to the alETH again and deposit it into the transmuter again. But this is not what sponsors have in mind. They have confirmed that they might not claim the whole balance because of the slippage issues. And also they were under the wrong impression that transmuter.getUnexchangedBalance(address(this)) fetches the balance which will also include the claimable balance and were hesitant about the double counting of the balance.

Also as per them, there could also be some potential delay between when claimAndSwap(...) is called and when report(...) is called. So even if the whole balance is claimed in the claimAndSwap(...) there are chances that there is new claimable balance for the time between that period. And that could also be potentially big enough if the amount is big. Because the default unlock period of the profit is 10 days and if the tokens are some big amount then we will have potentially big amount as a claimable again if new WETH comes into the contract.

So if claimable tokens are not taken into account then _harvestAndReport(...) will give the wrong total assets and there will losses or less profit will be shown.

POC:

function test_notClaimingAllBalanceWillCauseLoss() public {
uint256 _amount = 100e18;
​
// Deposit into strategy
mintAndDepositIntoStrategy(strategy, user, _amount);
console.log("Amount deposited:", _amount);
console.log("Total Assets:", strategy.totalAssets());
console.log("Claimable:", strategy.claimableBalance());
console.log("Unexchanged Balance:", strategy.unexchangedBalance());
console.log("Exchanged Balance:", transmuter.getExchangedBalance(address(strategy)));
console.log("Total Unexchanged:", transmuter.totalUnexchanged());
console.log("Total Buffered:", transmuter.totalBuffered());
​
assertApproxEq(strategy.totalAssets(), _amount, _amount / 500);
vm.roll(1);
​
deployMockYieldToken();
console.log("Deployed Mock Yield Token");
addMockYieldToken();
console.log("Added Mock Yield Token");
depositToAlchemist(_amount);
console.log("Deposited to Alchemist");
airdropToMockYield(_amount / 2);
console.log("Airdropped to Mock Yield");
​
vm.prank(whale);
asset.transfer(user2, _amount);
​
vm.prank(user2);
asset.approve(address(transmuter), _amount);
​
vm.prank(user2);
transmuter.deposit(_amount / 2, user2);
​
vm.roll(1);
harvestMockYield();
​
vm.prank(address(transmuterKeeper));
transmuterBuffer.exchange(address(underlying));
​
//Note : link to Transmuter tests https://github.com/alchemix-finance/v2-foundry/blob/reward-collector-fix/test/TransmuterV2.spec.ts
skip(7 days);
vm.roll(5);
​
vm.prank(user2);
transmuter.deposit(_amount / 2, user2);
​
vm.prank(address(transmuterKeeper));
transmuterBuffer.exchange(address(underlying));
​
console.log("Skip 7 days");
console.log("Claimable:", strategy.claimableBalance());
console.log("Unexchanged Balance:", strategy.unexchangedBalance());
console.log("Exchangable Balance:", transmuter.getExchangedBalance(address(strategy)));
console.log("Total Unexchanged:", transmuter.totalUnexchanged());
console.log("Total Buffered:", transmuter.totalBuffered());
​
assertGt(strategy.claimableBalance(), 0, "!claimableBalance");
assertEq(strategy.totalAssets(), _amount);
uint256 claimable = strategy.claimableBalance() / 2;
​
// we do this as oracle needs swap to be done recently
//smallCurveSwap();
​
skip(1 seconds);
vm.roll(1);
​
vm.prank(keeper);
​
if (block.chainid == 1) {
// Mainnet
IStrategyInterface(address(strategy)).claimAndSwap(claimable, claimable * 101 / 100, 0);
} else if (block.chainid == 10) {
// NOTE on OP we swap directly to WETH
IVeloRouter.route[] memory veloRoute = new IVeloRouter.route[]();
veloRoute[0] =
IVeloRouter.route(address(underlying), address(asset), true, 0xF1046053aa5682b4F9a81b5481394DA16BE5FF5a);
// Velo Iterface
console.log("we are reaching upto here");
IStrategyInterfaceVelo(address(strategy)).claimAndSwap(claimable, claimable * 101 / 100, veloRoute);
} else if (block.chainid == 42161) {
// ARB
// NOTE we swap first to eFrax and then to WETH
IRamsesRouter.route[] memory ramsesRoute = new IRamsesRouter.route[]();
address eFrax = 0x178412e79c25968a32e89b11f63B33F733770c2A;
ramsesRoute[0] = IRamsesRouter.route(address(underlying), eFrax, true);
ramsesRoute[1] = IRamsesRouter.route(eFrax, address(asset), true);
​
IStrategyInterfaceRamses(address(strategy)).claimAndSwap(claimable, claimable * 101 / 100, ramsesRoute);
} else {
revert("Chain ID not supported");
}
​
// check balances post swap
console.log("Claimable:", strategy.claimableBalance());
console.log("Unexchanged Balance:", strategy.unexchangedBalance());
console.log("Exchangable Balance:", transmuter.getExchangedBalance(address(strategy)));
console.log("Total Unexchanged:", transmuter.totalUnexchanged());
console.log("Total Assets in Strategy:", strategy.totalAssets());
console.log("Free Assets in Strategy:", asset.balanceOf(address(strategy)));
console.log("Underlying in Strategy:", underlying.balanceOf(address(strategy)));
​
vm.prank(keeper);
(uint256 profit, uint256 loss) = strategy.report();
​
console.log("Profit:", profit);
console.log("Loss:", loss);
​
// assertGt(strategy.totalAssets(), _amount, "!totalAssets");
​
// assertEq(strategy.totalAssets(), strategy.claimableBalance(), "Force Failure");
}

output:

Logs:
Amount deposited: 100000000000000000000
Total Assets: 100000000000000000000
Claimable: 0
Unexchanged Balance: 100000000000000000000
Exchanged Balance: 0
Total Unexchanged: 153402379593778333024
Total Buffered: 0
Deployed Mock Yield Token
Added Mock Yield Token
Deposited to Alchemist
Airdropped to Mock Yield
Skip 7 days
Claimable: 14749114095635038800
Unexchanged Balance: 85250885904364961200
Exchangable Balance: 14749114095635038800
Total Unexchanged: 246026822545960813624
Total Buffered: 0
we are reaching upto here
Claimable: 7374557047817519400
Unexchanged Balance: 92709592752220847249
Exchangable Balance: 7374557047817519400
Total Unexchanged: 238736415298181660873
Total Assets in Strategy: 100000000000000000000
Free Assets in Strategy: 0
Underlying in Strategy: 0
Profit: 0
Loss: 7290407247779152751

We can see the loss above due to not account for the claimable balance.

Impact

There are chances of potentially losses.

Tools Used

  • Manual Review

  • Foundry

Recommendations

It is recommended to take into account the claimable balance in _harvestAndReport(...) and use some oracles or exchange input for getting the quote for that claimable WETH to alETH. Then it will give the correct balacne.

Updates

Appeal created

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

Incorrect accounting in `_harvestAndReport` claimable should be included

balanceDeployed() and _harvestAndReport() add WETH and alETH, but they have different prices

strapontin Auditor
8 months ago
goran Auditor
8 months ago
bshyuunn Auditor
8 months ago
goran Auditor
8 months ago
strapontin Auditor
8 months ago
bshyuunn Auditor
8 months ago
strapontin Auditor
8 months ago
bshyuunn Auditor
8 months ago
inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect accounting in `_harvestAndReport` claimable should be included

balanceDeployed() and _harvestAndReport() add WETH and alETH, but they have different prices

Support

FAQs

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