Stratax Contracts

First Flight #57
Beginner FriendlyDeFi
100 EXP
Submission Details
Impact: medium
Likelihood: low

Unsafe `transfer()` In `recoverTokens()` Silently Fails For Non-Standard ERC20 Tokens

Author Revealed upon completion

The recoverTokens() function in Stratax.sol is the sole emergency recovery mechanism for tokens that accumulate in the contract during normal operations (e.g., swap dust, partial operations, or tokens sent directly). It calls IERC20(_token).transfer(owner, _amount) without checking the boolean return value and without using OpenZeppelin's SafeERC20.safeTransfer() wrapper.

// src/Stratax.sol
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
IERC20(_token).transfer(owner, _amount); // @audit return value not checked; non-standard tokens returning false will not cause a revert
}

The contract imports the standard IERC20 interface but does not import or use SafeERC20 anywhere:

// src/Stratax.sol
import {IERC20} from "forge-std/interfaces/IERC20.sol"; // @audit no SafeERC20 wrapper used in the contract

The Stratax contract interacts with arbitrary ERC20 tokens provided by users as collateral and borrow tokens. For the narrow class of non-standard ERC20 tokens that explicitly return false on failure rather than reverting, a failed transfer completes without reverting. The Solidity ^0.8.13 compiler decodes the bool return value but does not enforce that it is true when the return value is discarded. The function therefore returns successfully, giving the owner a false indication that recovery succeeded while the tokens remain stuck in the contract.

It is worth noting that USDT on Ethereum mainnet does not return a bool at all from its transfer() function. Under Solidity ^0.8 ABI decoding rules, calling IERC20(usdt).transfer() would revert due to a return data length mismatch, meaning USDT is not affected by the silent failure scenario. However, other non-standard tokens that do explicitly return false on failure remain affected.

The same unchecked return value pattern exists in createLeveragedPosition() with transferFrom(), and across all approve() calls throughout the contract. This is a systemic issue documented as SWC-104 (Unchecked Return Values For Calls To External Contracts).

This issue has a medium impact as the recoverTokens() function is the only mechanism to recover stuck tokens from the contract. If it silently fails, tokens remain stuck. However, all contract functions are restricted by onlyOwner, meaning no external attacker can exploit this to steal funds. The owner can observe the on-chain state (unchanged token balance) and diagnose the failure.

This issue has a low likelihood as it requires that a non-standard token which explicitly returns false on failure (rather than reverting or returning no data) is used with the contract, and that the transfer actually fails. The set of commonly-used ERC20 tokens on Aave that exhibit this behaviour and are likely to accumulate in this contract is narrow.

recommendation

Replace the raw transfer() call in recoverTokens() with OpenZeppelin's SafeERC20.safeTransfer(). Import SafeERC20 and apply the using SafeERC20 for IERC20 directive, then replace IERC20(_token).transfer(owner, _amount) with IERC20(_token).safeTransfer(owner, _amount). Apply the same SafeERC20 wrapper to all other ERC20 interactions in the contract, including the transferFrom() call in createLeveragedPosition() and all approve() calls, to ensure consistent safe handling of non-standard tokens.

Support

FAQs

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

Give us feedback!