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

Unchecked transfer return value in recoverTokens function

Author Revealed upon completion

Description

  • The recoverTokens function does not check the return value of the ERC20 transfer call. Some tokens (like ZRX, BNB) return false instead of reverting on failure, which causes the transfer to fail silently.

    In Stratax.sol:282-284, the recoverTokens function performs an ERC20 transfer without checking the return value:

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

    In Solidity 0.8+, calling transfer through the IERC20 interface ABI-decodes the return data as bool, but the decoded value is never checked. If the token returns false on failure, the function completes successfully while no tokens are actually transferred. The owner may believe tokens were recovered when they were not.

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

Risk

Likelihood:

  • Requires a token that returns false instead of reverting on failure

  • Most standard ERC20 tokens revert on failure, making this uncommon

  • Only affects the recoverTokens function which is used for emergency token recovery

Impact:

  • Owner may believe tokens were recovered when the transfer actually failed

  • Could delay or complicate emergency recovery efforts

  • No direct fund loss, but tokens remain stuck in the contract with no indication of failure

Proof of Concept

The following compilable Foundry test demonstrates that recoverTokens silently succeeds when the underlying transfer returns false:

Steps:

  1. Deploy a mock token that returns false on insufficient balance (instead of reverting)

  2. Mint 100 tokens to the Stratax contract

  3. Call recoverTokens requesting 1000 tokens (more than available)

  4. The call succeeds — no revert — but zero tokens are transferred

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
​
import "forge-std/Test.sol";
import "../src/Stratax.sol";
​
/// @notice ERC20 that returns false on transfer failure instead of reverting
contract FalseReturnToken {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
uint8 public decimals = 18;
​
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
}
​
function transfer(address to, uint256 amount) external returns (bool) {
if (balanceOf[msg.sender] < amount) {
return false; // Silent failure — no revert
}
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
​
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (balanceOf[from] < amount) return false;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
​
function approve(address, uint256) external returns (bool) { return true; }
}
​
contract UncheckedTransferTest is Test {
Stratax stratax;
FalseReturnToken token;
address deployer = address(this);
​
function setUp() public {
stratax = new Stratax();
stratax.initialize(
address(0x1), address(0x2), address(0x3), address(0x4), address(0x5)
);
token = new FalseReturnToken();
}
​
function test_RecoverTokensSilentFailure() public {
// Give the contract 100 tokens
token.mint(address(stratax), 100 ether);
assertEq(token.balanceOf(address(stratax)), 100 ether);
​
// Try to recover MORE than available — token returns false, not revert
stratax.recoverTokens(address(token), 1000 ether);
​
// Function succeeded but NO tokens were transferred
assertEq(token.balanceOf(address(stratax)), 100 ether, "Tokens still in contract");
assertEq(token.balanceOf(deployer), 0, "Owner received nothing");
}
​
function test_RecoverTokensNormalCase() public {
// Give the contract 100 tokens and recover the exact amount
token.mint(address(stratax), 100 ether);
stratax.recoverTokens(address(token), 100 ether);
​
// Normal case works correctly
assertEq(token.balanceOf(deployer), 100 ether, "Owner should receive tokens");
assertEq(token.balanceOf(address(stratax)), 0, "Contract should be empty");
}
}

Both tests pass, confirming the silent failure:

[PASS] test_RecoverTokensSilentFailure()
[PASS] test_RecoverTokensNormalCase()

Recommended Mitigation

Use OpenZeppelin's SafeERC20 library which handles both cases — tokens returning false and tokens returning no data (e.g., USDT):

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

Support

FAQs

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

Give us feedback!