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 11 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.

Give us feedback!