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

Missing Reentrancy Guards On Flash Loan Operations

Author Revealed upon completion

The Stratax contract is a leveraged position manager deployed behind a BeaconProxy, where each user receives their own proxy instance. It inherits from OpenZeppelin's Initializable but does not inherit ReentrancyGuardUpgradeable. None of the contract's public or external functions use a nonReentrant modifier or equivalent mutex lock. The contract interacts with Aave for flash loans, supply, borrow, repay, and withdraw operations; with 1inch for token swaps via low-level call; and with arbitrary ERC-20 tokens for approve, transfer, and transferFrom operations.

The affected entry points are createLeveragedPosition(), unwindPosition(), executeOperation(), and recoverTokens(). During execution, the flash loan callback executeOperation() triggers internal operations that make multiple external calls through _call1InchSwap(), Aave pool interactions, and ERC-20 token transfers. At each of these external call sites, control is transferred to an external contract, and no reentrancy guard prevents re-entry into the Stratax contract.

// src/Stratax.sol
function executeOperation(
address _asset,
uint256 _amount,
uint256 _premium,
address _initiator,
bytes calldata _params
) external returns (bool) { // @audit no nonReentrant modifier
require(msg.sender == address(aavePool), "Caller must be Aave Pool");
require(_initiator == address(this), "Initiator must be this contract");
​
OperationType opType = abi.decode(_params, (OperationType));
​
if (opType == OperationType.OPEN) {
return _executeOpenOperation(_asset, _amount, _premium, _params);
} else {
return _executeUnwindOperation(_asset, _amount, _premium, _params);
}
}
// src/Stratax.sol
function createLeveragedPosition(
// ... snip ...
) public onlyOwner { // @audit no nonReentrant modifier
require(_collateralAmount > 0, "Collateral Cannot be Zero");
IERC20(_flashLoanToken).transferFrom(msg.sender, address(this), _collateralAmount);
// ... snip ...
aavePool.flashLoanSimple(address(this), _flashLoanToken, _flashLoanAmount, encodedParams, 0);
}
// src/Stratax.sol
function _call1InchSwap(bytes memory _swapParams, address _asset, uint256 _minReturnAmount)
internal
returns (uint256 returnAmount)
{
(bool success, bytes memory result) = address(oneInchRouter).call(_swapParams); // @audit low-level call with user-supplied calldata, no reentrancy guard
require(success, "1inch swap failed");
​
if (result.length > 0) {
(returnAmount,) = abi.decode(result, (uint256, uint256));
} else {
returnAmount = IERC20(_asset).balanceOf(address(this)); // @audit balance-based fallback could be manipulated in a reentrancy scenario
}
require(returnAmount >= _minReturnAmount, "Insufficient return amount from swap");
return returnAmount;
}

In the current implementation, the contract does not maintain internal balance mappings, reward tracking, or shared position state. All entry points are protected by onlyOwner or msg.sender == aavePool checks, and each user operates on an isolated proxy instance. This means reentrancy is not currently exploitable, as an owner re-entering their own isolated proxy has no shared state to corrupt.

However, the onlyOwner modifier on Stratax.sol on line 158 contains a TODO comment stating that contract ownership will be handled in ERC721 style so that open positions will be transferrable. This planned upgrade would introduce shared state into the contract, and without reentrancy guards already in place, the existing external call paths through 1inch swaps, Aave callbacks, and ERC-20 transfers would become reentrancy vectors. Additionally, the balance-based fallback in _call1InchSwap() that relies on balanceOf(address(this)) rather than return data could be manipulated if multi-user accounting is introduced.

The contract uses Initializable from OpenZeppelin upgradeable contracts but does not use ReentrancyGuardUpgradeable, which is available in the same package and designed for this proxy pattern. All similar DeFi leverage and flash loan protocols apply reentrancy guards as standard practice.

This issue has a low impact as the contract does not currently maintain any internal state that could be corrupted via reentrancy, and the per-user proxy isolation model means there are no shared funds across users. The impact would escalate significantly if the planned ERC721-based position tracking or other shared state is introduced in a future upgrade.

This issue has a low likelihood as it is not currently exploitable due to the onlyOwner access control on all user-facing entry points, the msg.sender == aavePool check on executeOperation(), and the absence of any exploitable mutable state. The per-user BeaconProxy isolation model further limits the attack surface.

recommendation

Inherit from ReentrancyGuardUpgradeable (OpenZeppelin's upgradeable variant) and apply the nonReentrant modifier to createLeveragedPosition(), unwindPosition(), executeOperation(), and recoverTokens(). Initialise the reentrancy guard in the initialize() function by calling __ReentrancyGuard_init(). Use ReentrancyGuardUpgradeable rather than the non-upgradeable ReentrancyGuard to maintain compatibility with the BeaconProxy deployment pattern and avoid storage layout conflicts. Apply this change before implementing the planned ERC721 position tracking, as that upgrade will introduce shared state that creates a concrete reentrancy attack surface.

Support

FAQs

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

Give us feedback!