Uninitialized Implementation Contract Vulnerability
Description
The Stratax contract uses the Initializable pattern from OpenZeppelin but does not disable initializers in the constructor. This means the implementation contract itself can be initialized by anyone, potentially taking ownership. While this doesn't directly affect proxy deployments (which have their own state), if the implementation contract is used directly or holds any value, an attacker could take control. Additionally, uninitialized implementation contracts can sometimes be leveraged for further attacks or misleading users.
Risk
Likelihood:
Impact:
-
An attacker can become the owner of the implementation contract.
-
If the implementation contract is used directly (not through proxy), the attacker gains full control.
-
The attacker could set malicious protocol addresses in the implementation.
Proof of Concept
Stratax implementation = new Stratax();
implementation.initialize(
attackerControlledAavePool,
attackerControlledDataProvider,
attackerControlled1inch,
attackerControlledUSDC,
attackerControlledOracle
);
assert(implementation.owner() == attacker);
Recommended Mitigation
contract Stratax is Initializable {
+ /// @custom:oz-upgrades-unsafe-allow constructor
+ constructor() {
+ _disableInitializers();
+ }
function initialize(
address _aavePool,
address _aaveDataProvider,
address _oneInchRouter,
address _usdc,
address _strataxOracle
) external initializer {
// ... code ...
}
}
Decimal Mismatch Vulnerability in Token Operations
Description
The contract assumes it can safely call decimals() on any ERC20 token. However, decimals() is optional in the ERC20 standard. Some tokens (e.g., USDT on certain chains) do not implement this function, which would cause the transaction to revert. Additionally, the contract doesn't validate that token decimals are within reasonable bounds (e.g., 0-77), which could lead to overflow in calculations when dealing with tokens with unusual decimal values.
Risk
Likelihood:
Impact:
-
Transactions fail when interacting with tokens that don't implement decimals().
-
Potential overflow when calculating 10**decimals for tokens with large decimal values (>77).
-
DoS for certain token pairs that are otherwise compatible with Aave.
Proof of Concept
contract MaliciousToken {
function decimals() external pure returns (uint8) {
return 100;
}
}
Recommended Mitigation
// Add a safe decimals function
+ function _safeGetDecimals(address token) internal view returns (uint8) {
+ try IERC20(token).decimals() returns (uint8 decimals) {
+ require(decimals <= 77, "Decimals too large"); // 10^77 < 2^256
+ return decimals;
+ } catch {
+ // Default to 18 if decimals() not implemented
+ // Or revert with clear error
+ revert("Token decimals not available");
+ }
+ }
// Use safe function in calculations:
- uint256 collateralToWithdraw = (
- _amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals()) * LTV_PRECISION
- ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * liqThreshold);
+ uint8 collateralDecimals = _safeGetDecimals(unwindParams.collateralToken);
+ uint8 assetDecimals = _safeGetDecimals(_asset);
+ uint256 collateralToWithdraw = (
+ _amount * debtTokenPrice * (10 ** uint256(collateralDecimals)) * LTV_PRECISION
+ ) / (collateralTokenPrice * (10 ** uint256(assetDecimals)) * liqThreshold);
Insufficient Validation of 1inch Swap Return Values
Description
The _call1InchSwap function attempts to decode the return value as (uint256, uint256) but doesn't validate that the returned data is actually the expected size. If the 1inch router returns data in a different format or returns less data than expected, abi.decode could fail silently or decode garbage values. Falling back to checking balance when result.length is 0 is unreliable as it includes any existing balance, not just swap proceeds.
Risk
Likelihood:
Impact:
-
Incorrect return amount calculation leading to improper flash loan repayment.
-
Potential for manipulation if an attacker can influence the contract's token balance before the swap.
-
Silent failures where swap doesn't execute but balance check passes.
Proof of Concept
Recommended Mitigation
function _call1InchSwap(bytes memory _swapParams, address _asset, uint256 _minReturnAmount)
internal
returns (uint256 returnAmount)
{
+ // Record balance before swap
+ uint256 balanceBefore = IERC20(_asset).balanceOf(address(this));
// Execute the 1inch swap using low-level call with the calldata from the API
(bool success, bytes memory result) = address(oneInchRouter).call(_swapParams);
require(success, "1inch swap failed");
- // Decode the return amount from the swap
- if (result.length > 0) {
- (returnAmount,) = abi.decode(result, (uint256, uint256));
- } else {
- // If no return data, check balance
- returnAmount = IERC20(_asset).balanceOf(address(this));
- }
+ // Calculate actual tokens received
+ uint256 balanceAfter = IERC20(_asset).balanceOf(address(this));
+ returnAmount = balanceAfter - balanceBefore;
+ // Verify we received tokens
+ require(returnAmount > 0, "No tokens received from swap");
// Sanity check
require(returnAmount >= _minReturnAmount, "Insufficient return amount from swap");
return returnAmount;
}
Missing Validation of Aave Pool Address Immutability
Description
The executeOperation callback only checks that msg.sender equals address(aavePool), but aavePool is a mutable state variable that can be changed by the owner through re-initialization (if not properly protected) or if the contract logic allows updates. While initialize is protected by initializer, having critical addresses as mutable storage variables poses a risk if the owner is compromised.
Risk
Likelihood:
Impact:
Proof of Concept
contract FakeAavePool {
function attackStratax(address stratax, bytes memory params) external {
Stratax(stratax).executeOperation(
collateralToken,
1000000 ether,
0,
stratax,
params
);
}
}
Recommended Mitigation
// Make critical addresses immutable if possible
contract Stratax is Initializable {
- IPool public aavePool;
+ IPool public immutable aavePool;
- IProtocolDataProvider public aaveDataProvider;
+ IProtocolDataProvider public immutable aaveDataProvider;
- IAggregationRouter public oneInchRouter;
+ IAggregationRouter public immutable oneInchRouter;
constructor(
address _aavePool,
address _aaveDataProvider,
address _oneInchRouter
) {
+ aavePool = IPool(_aavePool);
+ aaveDataProvider = IProtocolDataProvider(_aaveDataProvider);
+ oneInchRouter = IAggregationRouter(_oneInchRouter);
_disableInitializers();
}
function initialize(
address _usdc,
address _strataxOracle
) external initializer {
// Remove address args that are now immutable
USDC = _usdc;
strataxOracle = _strataxOracle;
owner = msg.sender;
flashLoanFeeBps = 9;
}
}
Precision Loss in Leverage Calculations
Description
The calculateOpenParams function performs multiple division operations that can lead to precision loss, particularly when dealing with tokens with different decimal places. The calculation of borrowValueUSD divides by (LTV_PRECISION * 10000) which results in rounding down. This precision loss is compounded when converting between tokens with different decimals, potentially resulting in positions that are under-leveraged or borrow amounts that don't fully utilize the available collateral.
Risk
Likelihood:
Impact:
-
Users receive less leverage than requested due to rounding errors.
-
Inefficient capital utilization.
-
Accumulated precision loss when dealing with tokens with very different decimal places (e.g., WBTC with 8 decimals vs WETH with 18).
Proof of Concept
Recommended Mitigation
function calculateOpenParams(TradeDetails memory details)
public
view
returns (uint256 flashLoanAmount, uint256 borrowAmount)
{
// ... previous code ...
+ // Use higher precision intermediate calculations
+ uint256 PRECISION = 1e27; // High precision constant
- // Calculate borrow value in USD (with 8 decimals)
- uint256 borrowValueUSD = (totalCollateralValueUSD * ltv * BORROW_SAFETY_MARGIN) / (LTV_PRECISION * 10000);
- // Convert borrow value to borrow token amount
- borrowAmount = (borrowValueUSD * (10 ** details.borrowTokenDec)) / details.borrowTokenPrice;
+ // Calculate with higher precision
+ uint256 totalCollateralValuePrecise =
+ (totalCollateral * details.collateralTokenPrice * PRECISION)
+ / (10 ** details.collateralTokenDec);
+ // Apply LTV and safety margin with high precision
+ uint256 borrowValuePrecise =
+ (totalCollateralValuePrecise * ltv * BORROW_SAFETY_MARGIN)
+ / (LTV_PRECISION * 10000);
+ // Convert to borrow token amount with minimal precision loss
+ borrowAmount =
+ (borrowValuePrecise * (10 ** details.borrowTokenDec))
+ / (details.borrowTokenPrice * PRECISION / PRICE_FEED_PREC);
+ // Round up for safety to ensure sufficient borrowing
+ if (borrowAmount * details.borrowTokenPrice < borrowValuePrecise) {
+ borrowAmount += 1;
+ }
// ... rest of function ...
}