Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Unchecked Return Values in ERC20 Transfers

0bingo76

Summary

The TokenManager contract assumes that _safe_transfer_from will revert on failure, which isn't guaranteed for all token implementations.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L233-L250

The contract assumes that _safe_transfer_from will revert on failure. However, some ERC20 tokens (especially older or non-standard implementations) might return false instead of reverting on failure.

function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
// ... other code ...
_safe_transfer_from(_token, _from, _to, _amount);
// ... balance checks ...
}

Impact

This vulnerability potentiallly could lead to system-wide failure if exploited at scale. If a non-reverting token is used that simply returns false on failure, the contract might assume a transfer was successful when it wasn't. This could lead to:

  • Incorrect balance tracking

  • Potential loss of funds

  • Inconsistent contract state

Tools Used

manual code review

Recommendations

Use OpenZeppelin's SafeERC20 library for all ERC20 interactions:

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract TokenManager {
using SafeERC20 for IERC20;
​
function _transfer(address _token, address _from, address _to, uint256 _amount, address _capitalPoolAddr) internal {
// ... other code ...
IERC20(_token).safeTransferFrom(_from, _to, _amount);
// ... balance checks ...
}
}

Implement additional checks for token transfers:

function initialize(address _wrappedNativeToken) external onlyOwner { ... }
​
function updateTokenWhiteListed(address[] calldata _tokens, bool _isWhiteListed) external onlyOwner { ... }
Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-weird-erc-20-return-boolean-Rescuable

I believe the issues and duplicates do not warrant low severity severity as even if the call to transfers returns false instead of reverting, there is no impact as it is arguably correct given there will be insufficient funds to perform a rescue/withdrawal. This will not affect `tillIn()` as there are explicit balance [checks that revert accordingly](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L255-L260) to prevent allowing creation of offers without posting the necessary collateral

Support

FAQs

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