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

`Stratax::recoverTokens` uses unsafe `transfer` which silently fails for non-standard ERC20 tokens like USDT

Author Revealed upon completion

Stratax::recoverTokens uses unsafe transfer which silently fails for non-standard ERC20 tokens like USDT

Description

Stratax::recoverTokens uses IERC20.transfer directly without checking the return value:

function recoverTokens(address _token, uint256 _amount) external onlyOwner {
@> IERC20(_token).transfer(owner, _amount);
}

Some widely-used ERC20 tokens — most notably USDT (Tether) — do not return a bool from transfer. When the Solidity IERC20 interface expects a bool return value and the token returns nothing, the call reverts at the ABI decoding step. This means Stratax::recoverTokens will always revert when attempting to recover USDT or any other non-standard token that omits the return value.

This is particularly relevant to Stratax because the protocol is deployed on Ethereum mainnet and interacts with Aave, where USDT is one of the most commonly used assets. If a user opens a leveraged position using USDT as collateral or borrow token, any USDT that ends up in the contract (from excess withdrawals, swap leftovers, or direct transfers) becomes permanently unrecoverable via Stratax::recoverTokens.

Risk

Likelihood:

  • USDT is one of the most widely used stablecoins on Aave, so positions involving USDT are common.

  • The issue only occurs when Stratax::recoverTokens is called with a non-standard token like USDT, and also when trying to call Stratax::openLeveragedPosition where the non-standard token is the collateral token. This will revert

Impact:

  • Tokens that don't conform to the standard ERC20 transfer return value cannot be recovered from the contract.

  • Stratax::recoverTokens is the only mechanism to retrieve tokens sitting in the contract, so these funds are permanently locked.

  • Trying to open a position where a non-standard token eg: USDT is the collateral token will revert

Proof Of Concept

Add the following constants to Constants.t.sol

contract ConstantsEtMainnet {
// ...
address public constant USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7;
​
// Chainlink price feeds
// ...
address public constant USDT_PRICE_FEED = 0x3E7d1eAB13ad0104d2750B8863b489D65364e32D;
}

Add the folliwing to the setUp() function in /test/fork/Stratax.t.sol

function setUp() public {
// ...
strataxOracle.setPriceFeed(USDT, USDT_PRICE_FEED);
// ...
}

Add the following test to /test/fork/Stratax.t.sol

function test_USDTRevertsOnOpenPosition() public {
uint256 collateralAmount = 1000 * 10 ** 6;
// Calculate open parameters for USDT/WETH position
(uint256 flashLoanAmount, uint256 borrowAmount) = stratax.calculateOpenParams(
Stratax.TradeDetails({
collateralToken: address(USDT),
borrowToken: address(WETH),
desiredLeverage: 30_000,
collateralAmount: collateralAmount,
collateralTokenPrice: 0,
borrowTokenPrice: 0,
collateralTokenDec: 6,
borrowTokenDec: 18
})
);
​
// Get 1inch swap data for WETH/USDT swap
(bytes memory openSwapData,) = get1inchSwapData(WETH, USDT, borrowAmount, address(stratax));
// Deal USDT to ownerTrader
deal(USDT, ownerTrader, collateralAmount);
​
// Attempt to open position
vm.startPrank(ownerTrader);
// Approve USDT to be spent by Stratax, we must call it like this because USDT does not return a bool from approve
(bool success, ) = address(USDT).call(abi.encodeWithSelector(IERC20.approve.selector, address(stratax), collateralAmount));
assertTrue(success, "Approve USDT to be spent by Stratax should succeed");
// Expect revert
vm.expectRevert();
stratax.createLeveragedPosition(
USDT, flashLoanAmount, collateralAmount, WETH, borrowAmount, openSwapData, (flashLoanAmount * 950) / 1000
);
vm.stopPrank();
}

Recommended Mitigation

Use OpenZeppelin's SafeERC20 library which handles non-standard return values:

+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Stratax is Initializable {
+ using SafeERC20 for IERC20;
​
// ...
​
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
- IERC20(_token).transfer(owner, _amount);
+ IERC20(_token).safeTransfer(owner, _amount);
}
​
function createLeveragedPosition(
address _flashLoanToken,
uint256 _flashLoanAmount,
uint256 _collateralAmount,
address _borrowToken,
uint256 _borrowAmount,
bytes calldata _oneInchSwapData,
uint256 _minReturnAmount
) public onlyOwner {
require(_collateralAmount > 0, "Collateral Cannot be Zero");
// Transfer the user's collateral to the contract
- IERC20(_flashLoanToken).transferFrom(msg.sender, address(this), _collateralAmount);
+ IERC20(_flashLoanToken).safeTransferFrom(msg.sender, address(this), _collateralAmount);
// ...
}
}

Support

FAQs

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

Give us feedback!