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

Inconsistent Asset Accounting Due to Missing _totalAssets Update in claimAndSwap Function

bshyuunn

01. Relevant GitHub Links

02. Summary

The claimAndSwap function in the Strategy contracts allows for claiming and swapping underlying assets to achieve gains. However, while this operation can increase the strategy’s funds, it does not update the internal accounting variable _totalAssets. As a result, the contract’s view of total assets becomes stale until report() is called, which can lead to incorrect share calculations for users who deposit or withdraw in the interim.

03. Vulnerability Details

When claimAndSwap is called, the contract claims from the transmuter, swaps to the synthetic asset (aleth), and redeposits it. This effectively increases the Strategy’s holdings. However, this gain is not reflected in the _totalAssets value used within share calculation functions.

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
require(_minOut > _amountClaim, "minOut too low");
​
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

This function successfully increases the contract’s asset balance. However, no direct call is made to update _totalAssets. The share calculations rely on _totalAssets as follows:

function _convertToShares(
StrategyData storage S,
uint256 assets,
Math.Rounding _rounding
) internal view returns (uint256) {
// Saves an extra SLOAD if totalAssets() is non-zero.
uint256 totalAssets_ = _totalAssets(S);
uint256 totalSupply_ = _totalSupply(S);
​
// If assets are 0 but supply is not PPS = 0.
if (totalAssets_ == 0) return totalSupply_ == 0 ? assets : 0;
​
return assets.mulDiv(totalSupply_, totalAssets_, _rounding);
}

If a user deposits or withdraws after claimAndSwap but before report() is called, the ratio of shares-to-assets is distorted. Neither the users who previously deposited nor the new investors are properly accounted for in terms of their share of the profits. Similarly, upon withdrawal, if report() is not called to update _totalAssets, users may not realize the gains made from the claimAndSwap operation.

This vulnerability also exists in StrategyArb and StrategyOp contracts due to similar logic.

03. Impact

  • incorrect Share Valuation: Depositors may receive shares that do not accurately reflect the true value of the underlying assets.

  • Unrealized Gains: Gains remain hidden until report() is called, potentially causing unfair distribution of profits.

  • Inaccurate Withdrawals: Withdrawals performed before calling report() might not capture the actual growth in the Strategy’s assets, disadvantaging users.

04. Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.18;
​
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";
​
import {TokenizedStrategy} from "@tokenized-strategy/TokenizedStrategy.sol";
​
interface Strategy is IStrategyInterface {
function balanceDeployed() external returns (uint256);
}
​
contract PocTest is Setup {
function setUp() public virtual override {
super.setUp();
}
​
function test_poc_claimandswap_dont_change_totalassets() public {
// deposit
uint256 deposit_amount = 10e18;
mintAndDepositIntoStrategy(strategy, user, deposit_amount);
console.log("Deposit assets", deposit_amount);
​
console.log("Before Exchange Strategy(address(strategy)).balanceDeployed(): ", Strategy(address(strategy)).balanceDeployed());
console.log("Before Exchange Unexchanged Balance: ", transmuter.getUnexchangedBalance(address(strategy)));
console.log("Before Exchange Exchangable Balance: ", transmuter.getExchangedBalance(address(strategy)));
console.log("Before Exchage TotalAssets: ", TokenizedStrategy(address(strategy)).totalAssets());
​
uint256 totalAssetsBefore = TokenizedStrategy(address(strategy)).totalAssets();
uint256 balanceDeployedBefore = Strategy(address(strategy)).balanceDeployed();
​
keeperExchange(deposit_amount);
​
// claimAndSwap
uint256 claimable = strategy.claimableBalance();
​
vm.prank(keeper);
strategy.claimAndSwap(claimable, claimable * 101 / 100, 0);
​
console.log("Before Exchange Strategy(address(strategy)).balanceDeployed(): ", Strategy(address(strategy)).balanceDeployed());
console.log("Before Exchange Unexchanged Balance: ", transmuter.getUnexchangedBalance(address(strategy)));
console.log("Before Exchange Exchangable Balance: ", transmuter.getExchangedBalance(address(strategy)));
console.log("Before Exchage TotalAssets: ", TokenizedStrategy(address(strategy)).totalAssets());
​
uint256 totalAssetsAfter = TokenizedStrategy(address(strategy)).totalAssets();
uint256 balanceDeployedAfter = Strategy(address(strategy)).balanceDeployed();
​
assertEq(totalAssetsBefore, totalAssetsAfter);
assertGe(balanceDeployedAfter, balanceDeployedBefore);
​
uint256 beforeAssetBalance = asset.balanceOf(user);
​
// redeem
vm.prank(user);
strategy.redeem(deposit_amount, user, user);
console.log("Withdraw assets", asset.balanceOf(user) - beforeAssetBalance);
}
​
function keeperExchange(uint256 _amount) public {
vm.roll(1);
​
deployMockYieldToken();
addMockYieldToken();
depositToAlchemist(_amount);
airdropToMockYield(_amount / 2);
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 : Keeper가 직접 exchange 하네
​
//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));
}
​
}
$ forge test --mt test_poc_claimandswap_dont_change_totalassets --fork-url https://rpc.ankr.com/eth -vvv
​
Ran 1 test for src/test/testpoc.t.sol:PocTest
[PASS] test_poc_claimandswap_dont_change_totalassets() (gas: 3765589)
Logs:
Before Exchange Strategy(address(strategy)).balanceDeployed(): 10000000000000000000
Before Exchange Unexchanged Balance: 10000000000000000000
Before Exchange Exchangable Balance: 0
Before Exchage TotalAssets: 10000000000000000000
Before Exchange Strategy(address(strategy)).balanceDeployed(): 10001251595926077985
Before Exchange Unexchanged Balance: 10001251595926077985
Before Exchange Exchangable Balance: 0
Before Exchage TotalAssets: 10000000000000000000
Withdraw asset.balanceOf(user) 10000000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 19.02s (15.51s CPU time)
​
Ran 1 test suite in 20.02s (19.20s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

05. Tools Used

Manual Code Review and Foundry

06. Recommended Mitigation

  • Immediate Reporting: Call report() immediately after claimAndSwap to ensure _totalAssets is updated and all share calculations remain accurate.

  • Inline Implementation: Alternatively, integrate the claimAndSwap logic into the _harvestAndReport function so that any gains are immediately reflected in the _totalAssets value. This would ensure that every asset-influencing action is consistently accounted for, preventing discrepancies in share calculations.

Updates

Lead Judging Commences

inallhonesty Lead Judge
7 months ago

Appeal created

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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