Stratax Contracts
First Flight #57
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

M02. recoverTokens Uses Unchecked transfer Return Value

adcdiii

Root + Impact

Description

  • recoverTokens calls IERC20(_token).transfer(owner, _amount) directly without checking the return value and without using SafeERC20. Some ERC-20 tokens (notably USDT on Ethereum mainnet) return false on failure instead of reverting. The transfer silently fails, the owner believes the recovery succeeded, and the tokens remain in the contract.

// src/Stratax.sol:282-284
​
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
// @> No return value check; no SafeERC20 usage
// For tokens like USDT that return false on failure, this silently does nothing
IERC20(_token).transfer(owner, _amount);
}

Risk

Likelihood:

  • USDT is an extremely common token that returns false on failure rather than reverting — it is a likely candidate for accidental contract deposits

  • Any amount overflow or balance shortfall (e.g., trying to recover more than the balance) produces a silent no-op

Impact:

  • The owner calls recoverTokens believing funds were recovered; the transaction succeeds with a Transfer event from the token that may not fire (USDT does not emit on failed transfers), leaving no on-chain evidence of failure

  • Tokens remain stuck in the contract with no indication of what went wrong

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";
import {IERC20} from "forge-std/interfaces/IERC20.sol";
import {ConstantsEtMainnet} from "../Constants.t.sol";
import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
​
// Simulates a token that returns false on failure without reverting (USDT behavior)
contract NonRevertingToken {
mapping(address => uint256) public balanceOf;
​
function transfer(address, uint256 amount) external returns (bool) {
if (balanceOf[msg.sender] < amount) {
return false; // silent failure, no revert
}
balanceOf[msg.sender] -= amount;
return true;
}
​
function decimals() external pure returns (uint8) { return 6; }
}
​
contract UncheckedTransferTest is Test, ConstantsEtMainnet {
Stratax stratax;
NonRevertingToken token;
address owner = address(0x123);
​
function setUp() public {
vm.createSelectFork(vm.envString("ETH_RPC_URL"));
​
Stratax impl = new Stratax();
UpgradeableBeacon beacon = new UpgradeableBeacon(address(impl), address(this));
​
bytes memory initData = abi.encodeWithSelector(
Stratax.initialize.selector,
AAVE_POOL, AAVE_PROTOCOL_DATA_PROVIDER,
address(0xdead), address(0), address(0)
);
BeaconProxy proxy = new BeaconProxy(address(beacon), initData);
stratax = Stratax(address(proxy));
stratax.transferOwnership(owner);
​
token = new NonRevertingToken();
// No tokens in contract — balance is 0
}
​
function test_silent_transfer_failure() public {
vm.prank(owner);
// @> Attempting to recover 100 tokens when contract holds 0
// USDT-style token returns false, but the call does NOT revert
stratax.recoverTokens(address(token), 100e6);
​
// Transaction succeeds, owner thinks recovery worked
// But owner's balance of token is still 0
assertEq(token.balanceOf(owner), 0, "Silent failure: tokens not transferred");
}
}

Recommended Mitigation

Use SafeERC20.safeTransfer which checks the return value and reverts on failure:

// src/Stratax.sol
​
+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
// Or use the upgradeable variant if contract needs upgradeable deps
​
function recoverTokens(address _token, uint256 _amount) external onlyOwner {
- IERC20(_token).transfer(owner, _amount);
+ SafeERC20.safeTransfer(IERC20(_token), owner, _amount);
}
Updates

Lead Judging Commences

izuman Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

WEIRD ERC20 Tokens

Currently there is no support for weird ERC20 tokens i.e. FOT tokens, missing return values, reentrancy etc.

Support

FAQs

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

Give us feedback!