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

Unchecked transfer Return Value in recoverTokens

Author Revealed upon completion

Root + Impact

Location: src/Stratax.sol:283

Description

  • recoverTokens is the owner's emergency function to retrieve tokens stuck in the contract. The ERC20 transfer return value is not checked.

  • Non-reverting ERC20 tokens, such as USDT, return false on transfer failure instead of reverting. The function treats the call as successful regardless of the return value.

// src/Stratax.sol:282-284
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
IERC20(_token).transfer(owner, _amount); // @> return value ignored — false silently discarded
}

Risk

Likelihood:

  • Tokens like USDT, BNB, and OMG return false on failure rather than reverting — a call with insufficient balance returns false without any revert

  • The function is called precisely in emergency scenarios where the token state may be irregular

Impact:

  • The owner believes tokens were recovered when they were not — emergency recovery silently fails

  • In an active exploit scenario, this delays the owner's ability to drain remaining funds to safety

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
​
import {Test} from "forge-std/Test.sol";
import {Stratax} from "../../src/Stratax.sol";
​
// Simulates a non-reverting ERC20 (like USDT) that returns false on transfer failure
contract NonRevertingToken {
mapping(address => uint256) public balanceOf;
function transfer(address, uint256) external pure returns (bool) {
return false; // silently fails
}
function mint(address to, uint256 amount) external { balanceOf[to] += amount; }
}
​
contract UncheckedTransferPoCTest is Test {
Stratax stratax;
NonRevertingToken token;
​
function setUp() public { /* deploy stratax proxy */ }
​
function test_recoverTokensSilentlyFails() public {
token = new NonRevertingToken();
token.mint(address(stratax), 1000e6);
​
uint256 ownerBalanceBefore = token.balanceOf(stratax.owner());
​
vm.prank(stratax.owner());
stratax.recoverTokens(address(token), 1000e6); // @> does not revert
​
// Owner balance unchanged — tokens never moved
assertEq(token.balanceOf(stratax.owner()), ownerBalanceBefore);
}
}

Recommended Mitigation

Use OpenZeppelin's SafeERC20.safeTransfer() which wraps the transfer call and explicitly reverts if the return value is false or if no return value is given. This covers both reverting and non-reverting ERC20 implementations uniformly.

+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
- IERC20(_token).transfer(owner, _amount);
+ SafeERC20.safeTransfer(IERC20(_token), owner, _amount);
}

Support

FAQs

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

Give us feedback!