Description
-
Normal behavior: unwindPosition should execute according to caller-provided unwind bounds and use a consistent amount model between planning and execution.
-
Issue: The callback ignores _collateralToWithdraw and _debtToken from user input, then recomputes a different collateralToWithdraw formula than the helper used by callers. This desynchronizes quote generation from execution and can break unwind reliability.
function unwindPosition(..., uint256 _collateralToWithdraw, address _debtToken, ...) external onlyOwner {
UnwindParams memory params = UnwindParams({
collateralToken: _collateralToken,
@> collateralToWithdraw: _collateralToWithdraw,
@> debtToken: _debtToken,
debtAmount: _debtAmount,
...
});
}
function calculateUnwindParams(...) public view returns (uint256 collateralToWithdraw, uint256 debtAmount) {
...
@> collateralToWithdraw = (debtTokenPrice * debtAmount * 10 ** IERC20(_collateralToken).decimals())
@> / (collateralTokenPrice * 10 ** IERC20(_borrowToken).decimals());
@> collateralToWithdraw = (collateralToWithdraw * 1050) / 1000;
}
function _executeUnwindOperation(...) internal returns (bool) {
...
@> uint256 collateralToWithdraw = (
@> _amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals()) * LTV_PRECISION
@> ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * liqThreshold);
@> withdrawnAmount = aavePool.withdraw(unwindParams.collateralToken, collateralToWithdraw, address(this));
}
Risk
Likelihood:
-
Reason 1 // Frontend/operator uses calculateUnwindParams to prepare 1inch calldata, then callback withdraws a different amount.
-
Reason 2 // Unwind execution occurs regularly during deleveraging and debt management operations.
Impact:
-
Impact 1 // Unwind can revert due to quote-amount mismatch, preventing risk reduction.
-
Impact 2 // Caller intent on bounds/assets is not enforced, weakening operational safety guarantees.
Proof of Concept
This PoC opens a controlled mock environment, compares the helper-calculated withdraw amount with the callback-computed amount, then executes unwindPosition. The test proves two things in one run: the two formulas disagree, and callback execution ignores the caller-provided _collateralToWithdraw.
function testPoC_UnwindUsesDifferentWithdrawAmountThanCalculatorAndIgnoresUserInput() public {
(uint256 helperCollateralToWithdraw,) = stratax.calculateUnwindParams(address(collateralToken), address(debtToken));
uint256 callbackCollateralToWithdraw = (debtAmount * 1e8 * 1e18 * 1e4) / (1e8 * 1e18 * 8000);
assertTrue(callbackCollateralToWithdraw != helperCollateralToWithdraw, "amounts should differ");
vm.prank(owner);
stratax.unwindPosition(
address(collateralToken),
helperCollateralToWithdraw,
address(debtToken),
debtAmount,
abi.encodeWithSelector(MockOneInchRouter.executeSwap.selector),
0
);
assertEq(pool.lastWithdrawAmount(), callbackCollateralToWithdraw);
assertTrue(pool.lastWithdrawAmount() != helperCollateralToWithdraw);
}
Recommended Mitigation
The fix is to remove the dual-source truth. Use one canonical unwind-amount function and pass/validate user intent through to callback execution. This keeps frontend quote generation, calldata, and on-chain execution aligned and auditable.
- Ignore `_collateralToWithdraw`/`_debtToken` and recompute internal amount with a separate formula.
+ Enforce caller-provided `_collateralToWithdraw` and `_debtToken` in callback execution.
+ Use one canonical formula shared by `calculateUnwindParams` and `_executeUnwindOperation`.
+ Add explicit bounds checks: revert when execution amount deviates from planned amount.