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

Balance check doesn't account for total strategy value

0xbrett8571

Summary

https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L92-L113

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));
// BUG: Balance check only compares asset balance delta, ignoring the underlying token reduction
// This allows value extraction by showing local profit while having overall strategy loss
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

The profitability check only measures the local change in asset balance (balAfter - balBefore), while ignoring the reduction in underlying token balance from the claim operation. This creates a false profit scenario where:

  1. The strategy claims X amount of underlying tokens

  2. Swaps them for Y amount of asset tokens

  3. Only checks if Y > X in asset terms

  4. Fails to account for the total value change including both asset and underlying balances

Vulnerability Details

from incorrect value accounting in the swap profitability check. The function:

  1. Claims WETH from the transmuter

  2. Swaps it for alETH

  3. Only verifies that received alETH > claimed WETH in nominal terms

  4. Fails to ensure the swap maintains or increases total strategy value

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
// Claims WETH from transmuter, reducing underlying balance
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// BUG: This check is insufficient as it only ensures minOut > claimed amount
// without considering the actual value relationship between WETH and alETH
require(_minOut > _amountClaim, "minOut too low");
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
uint256 balAfter = asset.balanceOf(address(this));
// BUG: accounting error - only checks local balance change
// Doesn't account for the value of WETH claimed from transmuter
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

Impact

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
// Step 1: Claims WETH, reducing strategy's underlying balance
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
// Step 2: Insufficient check - only compares nominal amounts
require(_minOut > _amountClaim, "minOut too low");
// Step 3: Performs swap
router.exchange(...);
uint256 balAfter = asset.balanceOf(address(this));
// Step 4: CRITICAL BUG - Local balance check ignores claimed WETH value
require((balAfter - balBefore) >= _minOut, "Slippage too high");
}

Recommendations

// StrategyMainnet.sol, StrategyOp.sol, StrategyArb.sol
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, uint256 _routeNumber) external onlyKeepers {
+ // Track total strategy value before operation
+ uint256 totalValueBefore = balanceDeployed();
​
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
- require(_minOut > _amountClaim, "minOut too low");
+ // Enforce minimum profitability threshold (e.g. 1% premium)
+ require(_minOut >= _amountClaim * 101 / 100, "Insufficient premium");
​
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");
​
+ // Verify total strategy value increased after swap
+ uint256 totalValueAfter = balanceDeployed();
+ require(totalValueAfter > totalValueBefore, "Must increase total value");
​
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
​
+ // Add premium validation function
+function validatePremium(uint256 amountIn, uint256 minOut) internal pure returns (bool) {
+ return minOut >= amountIn * 101 / 100;
+}
Updates

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] Keepers can execute swaps that appear profitable in isolation but actually decrease total strategy value

Support

FAQs

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