Root Cause + Impact

Two ERC20 operations use raw transfer/transferFrom without checking return values. Tokens that return false on failure instead of reverting (notably USDT) silently fail, so recoverTokens() loses funds and createLeveragedPosition() proceeds with zero collateral.

Description

The ERC20 standard specifies that transfer and transferFrom return bool. Some widely-used tokens (USDT, BNB, OMG) return false on failure instead of reverting. Two call sites in Stratax do not check the return value:

1. recoverTokens() at L283:

If the token returns false, the transfer silently fails. The admin thinks tokens were recovered but nothing moved.

2. createLeveragedPosition() at L325:

If the token returns false, no collateral is transferred. The function proceeds to initiate the flash loan with zero real collateral. The transaction will likely revert downstream in the Aave callback when the position is under-collateralized, but the error message is misleading.

Risk

Likelihood: Medium

USDT is the 3rd largest stablecoin by market cap and a common Aave collateral asset. The protocol does not restrict which ERC20 tokens can be used. If someone uses USDT as the flash loan token, both vulnerable paths are reachable.

Impact: Medium

In recoverTokens(), admin loses the ability to recover tokens of non-reverting types. In createLeveragedPosition(), the misleading downstream revert wastes gas and confuses users. No funds are permanently lost in the open path (the whole transaction reverts atomically), but the recovery path genuinely fails to move tokens.

Proof of Concept

N/A. The issue is visible by inspection: IERC20.transfer() returns bool per the ERC20 spec, and neither call site checks the return value. The behavior of USDT returning false instead of reverting is well-documented.

Recommended Mitigation

Use OpenZeppelin's SafeERC20 library, which reverts on false returns and also handles tokens that don't return a value (like USDT):