Description
The Treasury contract directly uses transfer and transferFrom methods of ERC20 tokens without properly checking their return values. Some ERC20 tokens (like USDT) don't revert on failure and instead return false, while others don't return any value at all. The current implementation assumes all transfers succeed, which could lead to silent failures and loss of funds.
Vulnerable code locations:
IERC20(token).transferFrom(msg.sender, address(this), amount);
IERC20(token).transfer(recipient, amount);
Impact
Silent failures of token transfers could lead to loss of funds
State updates could occur even when transfers fail
Accounting system could become out of sync with actual balances
Some ERC20 tokens (e.g., USDT) could fail silently
Fix Recommendation
Use OpenZeppelin's SafeERC20 library:
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
contract Treasury is ITreasury, AccessControl, ReentrancyGuard {
using SafeERC20 for IERC20;
function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
_balances[token] += amount;
_totalValue += amount;
emit Deposited(token, amount);
}
function withdraw(
address token,
uint256 amount,
address recipient
) external override nonReentrant onlyRole(MANAGER_ROLE) {
if (token == address(0)) revert InvalidAddress();
if (recipient == address(0)) revert InvalidRecipient();
if (_balances[token] < amount) revert InsufficientBalance();
_balances[token] -= amount;
_totalValue -= amount;
IERC20(token).safeTransfer(recipient, amount);
emit Withdrawn(token, amount, recipient);
}
}
Alternative manual check:
function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
bool success = IERC20(token).transferFrom(msg.sender, address(this), amount);
require(success, "Token transfer failed");
uint256 balanceBefore = IERC20(token).balanceOf(address(this));
uint256 balanceAfter = IERC20(token).balanceOf(address(this));
require(balanceAfter - balanceBefore == amount, "Transfer amount mismatch");
_balances[token] += amount;
_totalValue += amount;
emit Deposited(token, amount);
}
Tools Used
Manual code review
Static analysis
Mock contract testing
