Submission Details
Severity:
The claimable balance from the Transmuter is not included in the total assets calculation.
Summary
Not including the claimable balance in the calculation of the total assets in harvest and report might make the tokenized strategy be at loss, when it actually is not.
Vulnerability Details
To calculate the total assets in the tokenized strategy, the method report() is called by the keeper. This function calls _harvestAndReport() which should calculate and return the amount of assets the strategy is managing.
The current implementation expects that the keeper will execute claimAndSwap() before calling the report() function, so that the underlying amount from the Transmuter is claimed, exchanged via DEX and deposited again into the Transmuter.
However, two edge cases can cause incorrect asset calculations:
Case 1:
If the Transmuter receive new funds from the Alchemist right between the claimAndSwap() and the report(), the strategy will have claimable balance that is not re-deposited, and is not accounted for in the _harvestAndReport().
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;
}
Case 2:
The claimAndSwap() will revert whenever the ration alAsset:underlying token is not at a premium. However the docs states that:
alAssets typically trade at a discount to the underlying, reflecting their representation of future value.
This shows that there is a case when the assets are not traded at a discount, so the claimAndSwap() will not be able to convert the claimable underlying token to alAsset and re-deposit it. Also, the strategy will keep reporting less and less total assets, until the claimAndSwap() is executed successfully.
In both cases is visible that not including the claimable assets in the calculation of the total assets in _harvestAndReport(). This results in inaccurate total asset calculations, causing the strategy to appear at a loss when it is not. This causes new depositors to buy at a discount while withdrawing users lose assets. Moreover, when the ratio is within the desired bounds once again, the new depositors will receive parts of the old depositor's yield.
Impact
The strategy underreports total assets, causing existing users to lose yield while allowing new depositors to enter at an artificial discount.
PoC
Case 1 - Alchemist deposit between claimAndSwap() and report().
import "forge-std/console.sol";
import {Setup, ERC20, IStrategyInterface} from "./utils/Setup.sol";
import {IStrategyInterfaceVelo} from "../interfaces/IStrategyInterface.sol";
import {IStrategyInterfaceRamses} from "../interfaces/IStrategyInterface.sol";
import {IVeloRouter} from "../interfaces/IVelo.sol";
import {IRamsesRouter} from "../interfaces/IRamses.sol";
contract PoCTest is Setup {
function setUp() public virtual override {
super.setUp();
deployMockYieldToken();
addMockYieldToken();
}
function claimAndSwap(uint256 claimable) public {
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
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");
}
}
function claimAndSwapThatReverts() public view {
console.log("price should be better than 1:1");
}
function alchemistDepositYieldIntoTransmuter(uint256 amount) public {
airdrop(underlying, address(transmuterBuffer), amount);
vm.prank(address(transmuterKeeper));
transmuterBuffer.exchange(address(underlying));
}
function logStrategyData() public view {
console.log("------------------Strategy Data------------------");
console.log("Total Assets:", strategy.totalAssets());
console.log("Claimable:", strategy.claimableBalance());
console.log("Unexchanged Balance:", strategy.unexchangedBalance());
console.log("Exchangable Balance:", transmuter.getExchangedBalance(address(strategy)));
console.log("\n");
}
function testPoC() public {
// Deposit
uint256 amount = 1 ether;
mintAndDepositIntoStrategy(strategy, user, amount);
console.log("Amount deposited:", amount);
logStrategyData();
// Mock alchemist harvesting yield and depositing it in transmuterBuffer and then calling exchange
alchemistDepositYieldIntoTransmuter(amount);
// Show that there is balance to be claimed
console.log("Amount deposited:", amount);
logStrategyData();
// Swap
uint256 claimableBalance = strategy.claimableBalance();
claimAndSwap(claimableBalance);
logStrategyData();
// Mock alchemist harvesting yield and depositing it in transmuterBuffer and then calling exchange
alchemistDepositYieldIntoTransmuter(amount);
// Harvest and report
vm.prank(keeper);
strategy.report();
logStrategyData();
// Show that it is less than before the exchange is executed.
}
}
Logs:
Amount deposited: 1000000000000000000
------------------Strategy Data------------------
Total Assets: 1000000000000000000
Claimable: 0
Unexchanged Balance: 1000000000000000000
Exchangable Balance: 0
Amount deposited: 1000000000000000000
------------------Strategy Data------------------
Total Assets: 1000000000000000000
Claimable: 22526036641519837
Unexchanged Balance: 977473963358480163
Exchangable Balance: 22526036641519837
------------------Strategy Data------------------
Total Assets: 1000000000000000000
Claimable: 0
Unexchanged Balance: 1000296141934171802
Exchangable Balance: 0
------------------Strategy Data------------------
Total Assets: 977763584701119255
Claimable: 22532557233052547
Unexchanged Balance: 977763584701119255
Exchangable Balance: 22532557233052547
Case 2 - The claimAndSwap() revert, because of the unaccebtable price of the alAsset
import "forge-std/console.sol";
import {Setup, ERC20, IStrategyInterface} from "./utils/Setup.sol";
import {IStrategyInterfaceVelo} from "../interfaces/IStrategyInterface.sol";
import {IStrategyInterfaceRamses} from "../interfaces/IStrategyInterface.sol";
import {IVeloRouter} from "../interfaces/IVelo.sol";
import {IRamsesRouter} from "../interfaces/IRamses.sol";
contract PoCTest is Setup {
function setUp() public virtual override {
super.setUp();
deployMockYieldToken();
addMockYieldToken();
}
function claimAndSwap(uint256 claimable) public {
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
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");
}
}
function claimAndSwapThatReverts() public view {
console.log("price should be better than 1:1");
}
function alchemistDepositYieldIntoTransmuter(uint256 amount) public {
airdrop(underlying, address(transmuterBuffer), amount);
vm.prank(address(transmuterKeeper));
transmuterBuffer.exchange(address(underlying));
}
function logStrategyData() public view {
console.log("------------------Strategy Data------------------");
console.log("Total Assets:", strategy.totalAssets());
console.log("Claimable:", strategy.claimableBalance());
console.log("Unexchanged Balance:", strategy.unexchangedBalance());
console.log("Exchangable Balance:", transmuter.getExchangedBalance(address(strategy)));
console.log("\n");
}
function testPoC() public {
// If the peg is 1:1 the vault will register loses
// Deposit
uint256 amount = 1 ether;
mintAndDepositIntoStrategy(strategy, user, amount);
console.log("Amount deposited:", amount);
logStrategyData();
for (uint256 i = 1; i <= 3; ++i) {
console.log("------------------Iteration", i, "------------------");
// Mock alchemic harvesting yield and depositing it in transmuterBuffer and then calling exchange
alchemistDepositYieldIntoTransmuter(amount);
// Show that there is balance to be claimed
logStrategyData();
// Don't Swap - simulate swap is reverting
claimAndSwapThatReverts();
// Harvest and report will be inaccurate, because of that and the users will register loses because the claimable is not included
vm.prank(keeper);
strategy.report();
logStrategyData();
}
}
}
Logs:
Amount deposited: 1000000000000000000
------------------Strategy Data------------------
Total Assets: 1000000000000000000
Claimable: 0
Unexchanged Balance: 1000000000000000000
Exchangable Balance: 0
------------------Iteration 1 ------------------
------------------Strategy Data------------------
Total Assets: 1000000000000000000
Claimable: 22526036641519837
Unexchanged Balance: 977473963358480163
Exchangable Balance: 22526036641519837
price should be better than 1:1
------------------Strategy Data------------------
Total Assets: 977473963358480163
Claimable: 22526036641519837
Unexchanged Balance: 977473963358480163
Exchangable Balance: 22526036641519837
------------------Iteration 2 ------------------
------------------Strategy Data------------------
Total Assets: 977473963358480163
Claimable: 45052073283039674
Unexchanged Balance: 954947926716960326
Exchangable Balance: 45052073283039674
price should be better than 1:1
------------------Strategy Data------------------
Total Assets: 954947926716960326
Claimable: 45052073283039674
Unexchanged Balance: 954947926716960326
Exchangable Balance: 45052073283039674
------------------Iteration 3 ------------------
------------------Strategy Data------------------
Total Assets: 954947926716960326
Claimable: 67578109924559511
Unexchanged Balance: 932421890075440489
Exchangable Balance: 67578109924559511
price should be better than 1:1
------------------Strategy Data------------------
Total Assets: 932421890075440489
Claimable: 67578109924559511
Unexchanged Balance: 932421890075440489
Exchangable Balance: 67578109924559511
Tools Used
Manual review.
Recommendations
Include the claimable assets in the calculation in _harvestAndReport() in all strategies.
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;
+ _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
Updates
Appeal created
inallhonesty 10 months ago
Submission Judgement Published Assigned finding tags:
Incorrect accounting in `_harvestAndReport` claimable should be included
Prize pool breakdown
Total prize
16,653 OP
nSLOC
25366 OP / LOC
15,800
853
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.