Stratax Contracts
First Flight #57
Beginner FriendlyDeFi
100 EXP
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

Unwind position ignores collateralToWithdraw param and uses liquidationThreshold instead of LTV

Author Revealed upon completion

Description

_executeUnwindOperation() contains two bugs:

  1. Ignored parameter: The user-provided _collateralToWithdraw is packed into UnwindParams, encoded, transmitted through the flash loan, decoded — and then never read. A new local variable collateralToWithdraw is declared at L575 that shadows the struct field, making the user parameter dead code.

  2. Wrong Aave parameter: The internal recalculation reads liquidationThreshold (return position 2) from getReserveConfigurationData() where it should read ltv (return position 1) — inconsistent with calculateOpenParams() (L386) which correctly reads ltv. The developer's own comments confirm the intent was to use LTV: the comment at L565 says "Get LTV" and the formula comment at L574 says "/ ltv", but the code reads liqThreshold.


Bug 1 — Parameter ignored:

The UnwindParams struct (L41-54) includes collateralToWithdraw (L45). The user provides this value to unwindPosition() (L238), it's stored at L246, encoded at L253, and decoded at L556. But _executeUnwindOperation() never reads unwindParams.collateralToWithdraw — it declares a new local variable with the same name at L575:

// L556: unwindParams is decoded, including unwindParams.collateralToWithdraw
(, address user, UnwindParams memory unwindParams) = abi.decode(_params, ...);
​
// ...
​
// L575-577: NEW local variable shadows the struct field — user param is ignored
@> uint256 collateralToWithdraw = (
@> _amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals()) * LTV_PRECISION
@> ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * liqThreshold);
​
// L579: Uses the recalculated value, not the user's
@> withdrawnAmount = aavePool.withdraw(unwindParams.collateralToken, collateralToWithdraw, address(this));

Bug 2 — Wrong Aave field:

IProtocolDataProvider.getReserveConfigurationData() returns (from IProtocolDataProvider.sol:L28-37):

Position Field Example (WETH)
0 decimals 18
1 ltv 8000 (80%)
2 liquidationThreshold 8250 (82.5%)
3 liquidationBonus 10500
... ... ...

Position creation (calculateOpenParams, L386) correctly reads position 1:

(, uint256 ltv,,,,,,,,) = aaveDataProvider.getReserveConfigurationData(details.collateralToken);
// ^ skips 0, reads 1 (ltv) ✓

Position unwinding (_executeUnwindOperation, L566-567) reads position 2 instead:

@> // Get LTV from Aave for the collateral token ← comment says "LTV"
@> (,, uint256 liqThreshold,,,,,,,) = ← but reads position 2 (liquidationThreshold)
@> aaveDataProvider.getReserveConfigurationData(unwindParams.collateralToken);

Since liquidationThreshold > ltv, the formula divides by a larger number, withdrawing less collateral than needed.

Risk

Likelihood:

  • Bug 2 triggers on every unwindPosition() call — the wrong Aave field is always used

  • Bug 1 means the user has no way to override the incorrect calculation

  • No special conditions required — it's a permanent code defect

Impact:

  • Insufficient collateral withdrawn: For WETH (ltv=8000, liqThreshold=8250), the function withdraws 3.03% less collateral than the position requires. For assets with larger ltv/liqThreshold gaps, the shortfall is greater.

  • DoS on unwind: Less collateral → swap produces fewer debt tokens → flash loan repayment fails at L588 (require(returnAmount >= totalDebt, "Insufficient funds to repay flash loan")) → transaction reverts. The user cannot unwind their position unless market conditions are favorable enough to absorb the 3% shortfall.

  • No user workaround: Because _collateralToWithdraw is ignored (Bug 1), the user cannot manually specify a larger withdrawal amount to compensate.

Proof of Concept

Two tests proving both bugs. Verified: both pass with forge test --match-path test/UnwindBugs.t.sol -vvv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
​
import {Test} from "forge-std/Test.sol";
import {Stratax} from "../src/Stratax.sol";
import {IProtocolDataProvider} from "../src/interfaces/external/IProtocolDataProvider.sol";
​
contract UnwindBugsTest is Test {
​
/// @notice Bug 2: liquidationThreshold (position 2) is used where ltv (position 1) should be.
/// This causes 3.03% less collateral to be withdrawn for WETH.
function test_liqThresholdUsedInsteadOfLtv() public pure {
// IProtocolDataProvider.getReserveConfigurationData returns:
// position 0: decimals
// position 1: ltv ← calculateOpenParams reads this (L386)
// position 2: liquidationThreshold ← _executeUnwindOperation reads this (L566)
//
// These are DIFFERENT fields. The code comment at L565 says "Get LTV"
// and the formula comment at L574 says "/ ltv", but the code reads position 2.
​
uint256 ltv = 8000; // 80% — used during position creation
uint256 liqThreshold = 8250; // 82.5% — incorrectly used during unwind
uint256 LTV_PRECISION = 1e4;
​
uint256 debtValueUsd = 10_000e8; // $10,000 in debt to repay
​
// Correct (using ltv): collateral = $10,000 × 10000/8000 = $12,500
uint256 correctCollateralValue = (debtValueUsd * LTV_PRECISION) / ltv;
assertEq(correctCollateralValue, 12_500e8);
​
// Actual (using liqThreshold): collateral = $10,000 × 10000/8250 ≈ $12,121
uint256 actualCollateralValue = (debtValueUsd * LTV_PRECISION) / liqThreshold;
assertEq(actualCollateralValue, 12_121_21212121); // ~$12,121.21
​
// Shortfall: ~$378.79 less collateral withdrawn (3.03%)
uint256 shortfall = correctCollateralValue - actualCollateralValue;
assertGt(shortfall, 378e8);
​
// This means the swap receives 3% less collateral → may not produce
// enough debt tokens to repay flash loan → tx reverts at L588:
// require(returnAmount >= totalDebt, "Insufficient funds to repay flash loan")
}
​
/// @notice Bug 1: User-provided collateralToWithdraw is packed, encoded,
/// decoded, then NEVER READ. A new local variable shadows it at L575.
function test_collateralToWithdrawParamIgnored() public pure {
// User provides _collateralToWithdraw = 999_999 ETH to unwindPosition()
uint256 userProvidedAmount = 999_999e18;
​
// But _executeUnwindOperation recalculates from scratch at L575-577.
// Simulating the internal calculation:
uint256 _amount = 1000e6; // flash loan: 1000 USDC
uint256 debtTokenPrice = 1e8; // $1 (USDC)
uint256 collateralTokenPrice = 2000e8; // $2000 (ETH)
uint256 collateralDec = 18;
uint256 debtDec = 6;
uint256 liqThreshold = 8250;
uint256 LTV_PRECISION = 1e4;
​
// Reproducing the formula at L575-577:
uint256 internalCalc = (
_amount * debtTokenPrice * (10 ** collateralDec) * LTV_PRECISION
) / (collateralTokenPrice * (10 ** debtDec) * liqThreshold);
​
// The internally calculated amount is completely different from user's input.
// User's 999_999e18 is silently discarded — the struct field is dead code.
assertTrue(internalCalc != userProvidedAmount, "User param is ignored");
}
}

Recommended Mitigation

Two fixes are needed — one for each bug:

Fix 1: Either use the user-provided parameter or remove the dead field from the struct:

Option A — Use the parameter (gives user control over partial unwinds):

- uint256 collateralToWithdraw = (
- _amount * debtTokenPrice * ... * LTV_PRECISION
- ) / (collateralTokenPrice * ... * ltv);
+ uint256 collateralToWithdraw = unwindParams.collateralToWithdraw;

Option B — Remove the dead field from UnwindParams and unwindPosition() if internal calculation is preferred (after fixing Bug 1).

Either approach eliminates the dead code and makes the API honest about what it actually does.

Fix 2: Read ltv (position 1) instead of liquidationThreshold (position 2), matching calculateOpenParams():

// Step 2: Calculate and withdraw only the collateral that backed the repaid debt
uint256 withdrawnAmount;
{
- // Get LTV from Aave for the collateral token
- (,, uint256 liqThreshold,,,,,,,) =
+ // Get LTV from Aave for the collateral token
+ (, uint256 ltv,,,,,,,,) =
aaveDataProvider.getReserveConfigurationData(unwindParams.collateralToken);
​
// Get prices and decimals
uint256 debtTokenPrice = IStrataxOracle(strataxOracle).getPrice(_asset);
uint256 collateralTokenPrice = IStrataxOracle(strataxOracle).getPrice(unwindParams.collateralToken);
require(debtTokenPrice > 0 && collateralTokenPrice > 0, "Invalid prices");
​
// Calculate collateral to withdraw: (debtAmount * debtPrice * collateralDec * LTV_PRECISION) / (collateralPrice * debtDec * ltv)
uint256 collateralToWithdraw = (
_amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals()) * LTV_PRECISION
- ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * liqThreshold);
+ ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * ltv);
​
withdrawnAmount = aavePool.withdraw(unwindParams.collateralToken, collateralToWithdraw, address(this));
}

Support

FAQs

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

Give us feedback!