DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Invalid

When funds are deposited via transmuter.deposit(), they affect the unexchanged balance but the accounting doesn't properly track this transformation

Summary

Incorrect balance accounting during state transitions in the transmuter deposit process

function _deployFunds(uint256 _amount) internal override {
// @check Double counting vulnerability: The deposit affects both unexchanged and underlying balances
// but balanceDeployed() counts both, leading to inflated total balance
transmuter.deposit(_amount, address(this));
}
function balanceDeployed() public view returns (uint256) {
// @check Balance inflation: Adding both unexchanged and underlying balances creates double counting
// since deposited funds appear in both balances during state transitions
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}
  1. When funds are deposited via transmuter.deposit(), they simultaneously appear in:

    • unexchangedBalance (via transmuter)

    • underlying.balanceOf (not immediately cleared)

  2. The balanceDeployed() function adds both these balances together, effectively counting the same funds twice during the deposit state transition.

This is a state transition vulnerability where the contract fails to properly account for the intermediate state where funds exist in multiple accounting buckets simultaneously.

The actual balance change after deployment doesn't match the expected amount, due to this double counting during the state transition period.

Vulnerability Details

In all strategy contracts (StrategyArb.sol, StrategyMainnet.sol, StrategyOp.sol), the vulnerability lies in the balance accounting during the deposit and claim process.

function _deployFunds(uint256 _amount) internal override {
// @check Funds are deposited into transmuter but remain counted in underlying balance
transmuter.deposit(_amount, address(this));
}
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
// @check Claimed funds are double counted - they exist in both underlying and unexchanged balances
transmuter.claim(_amountClaim, address(this));
// ...
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
function balanceDeployed() public view returns (uint256) {
// @check accounting error: Adds balances that can overlap during state transitions
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

The fundamental issue is in the state transition handling between these token states

  1. WETH (underlying) -> alETH (asset) conversion

  2. Unexchanged balance in transmuter

  3. Claimable balance from transmuter

The balanceDeployed() function adds all these balances together, but during state transitions (deposit/claim/swap), the same value can exist in multiple states simultaneously, leading to inflated balance reporting.

Impact

  1. Share Price Manipulation: Incorrect total assets calculation affects share price

  2. Accounting Errors: Strategy reports more assets than actually controlled

  3. Cross-chain Implications: Affects all implementations (Arbitrum, Optimism, Mainnet) due to shared architecture

Specific Vulnerabilities

interface ITransmuter {
// @check State transition vulnerability between these balances
function getClaimableBalance(address _owner) external view returns (uint256);
function getUnexchangedBalance(address _owner) external view returns (uint256);
}

The issue affects all supported tokens:

  • WETH <-> alETH conversions

  • Yearn V3 tokenized strategy accounting

This particularly affects the core accounting mechanism across all three blockchain implementations, potentially impacting the entire Alchemix ecosystem's balance reporting accuracy.

function _deployFunds
function _deployFunds
function _deployFunds
function claimAndSwap
function claimAndSwap
function claimAndSwap
function balanceDeployed
function balanceDeployed
function balanceDeployed

ITransmuter

Impact

As th bug is caused by incorrect balance accounting during state transitions in the transmuter system.

function balanceDeployed() public view returns (uint256) {
// @check Three-way balance addition creates overlap during transitions
return transmuter.getUnexchangedBalance(address(this)) +
underlying.balanceOf(address(this)) +
asset.balanceOf(address(this));
}

Deposit Flow Issue

function _deployFunds(uint256 _amount) internal override {
// @check Deposit creates temporary state where funds exist in multiple balances
transmuter.deposit(_amount, address(this));
}

Claim and Swap Flow Issue

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
// @check Multiple balance state changes without proper accounting resolution
transmuter.claim(_amountClaim, address(this));
// swap execution
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

The root cause is the lack of atomic state transitions in the balance accounting system. When funds move between states (underlying -> transmuter -> asset), they temporarily exist in multiple balance categories simultaneously. The balanceDeployed() function adds these balances together without accounting for these transition states, leading to double counting.

This also affects all three implementations (StrategyArb.sol, StrategyMainnet.sol, StrategyOp.sol) because they share the same fundamental accounting structure and interaction pattern with the transmuter system.

Recommendations

// In StrategyArb.sol, StrategyMainnet.sol, and StrategyOp.sol
// @Recommendation - Track state transitions with explicit balance states
+ enum TokenState { UNDERLYING, UNEXCHANGED, CLAIMED }
+ mapping(TokenState => uint256) private balanceStates;
- function balanceDeployed() public view returns (uint256) {
- return transmuter.getUnexchangedBalance(address(this)) +
- underlying.balanceOf(address(this)) +
- asset.balanceOf(address(this));
- }
+ function balanceDeployed() public view returns (uint256) {
+ // @Recommendation - Single source of truth for balance accounting
+ return transmuter.getUnexchangedBalance(address(this)) +
+ asset.balanceOf(address(this));
+ }
- function _deployFunds(uint256 _amount) internal override {
- transmuter.deposit(_amount, address(this));
- }
+ function _deployFunds(uint256 _amount) internal override {
+ // @Recommendation - Atomic state transition with balance tracking
+ uint256 preBalance = balanceDeployed();
+ transmuter.deposit(_amount, address(this));
+ require(balanceDeployed() == preBalance + _amount, "Invalid balance transition");
+ }
+ function claimAndSwap(uint256 _amountClaim, uint256 _minOut, ...) external onlyKeepers {
+ // @Recommendation - Track balance state changes during transitions
+ uint256 preBalance = balanceDeployed();
+ transmuter.claim(_amountClaim, address(this));
+ _swapUnderlyingToAsset(...);
+ transmuter.deposit(asset.balanceOf(address(this)), address(this));
+ require(balanceDeployed() >= preBalance, "Balance consistency check failed");
+ }

In this mitigations we implement

  1. Explicit Balance State Tracking:

  • Introduces state enumeration for different token states

  • Maintains clear separation between underlying, unexchanged, and claimed balances

  1. Atomic Balance Transitions:

  • Adds pre and post balance checks for state transitions

  • Ensures balance consistency during deposit and claim operations

  1. Single Source of Truth:

  • Simplifies balanceDeployed() to prevent double counting

  • Removes underlying balance from total calculation

  1. Balance Consistency Checks:

  • Adds requirements to verify balance transitions

  • Prevents invalid state transitions during operations

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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