Protocol does not check return value of ERC20 swaps
Description
The ERC20 standard requires that transfer operations return a boolean success value indicating whether the operation was successful or not. Therefore, it is important to check the return value of the transfer function before assuming that the transfer was successful. This helps ensure that the transfer was executed correctly and helps avoid potential issues with lost or mishandled funds.
Impact
The protocol's internal accounting will record failed transfer operations as a success if the underlying ERC20 token does not revert on failure.
Recommendations
We recommend implementing one of the following solutions to ensure that ERC20 transfers are handled securely:
Utilize OpenZeppelin's
SafeERC20
transfer methods, which provide additional checks and safeguards to ensure the safe handling of ERC20 transfers.Strictly whitelist ERC20 coins that do not return false on failure and revert. This will ensure that only safe and reliable ERC20 tokens are used within the protocol.
In general, it is important to exercise caution when integrating third-party tokens into the protocol. Tokens with hooks and atypical behaviors of the ERC20 standard can present security vulnerabilities that may be exploited by attackers. We recommend thoroughly researching and reviewing any tokens that are considered for integration and performing a comprehensive security review of the entire system to identify and mitigate any potential vulnerabilities.
Remediation
STFX acknowledged and resolved the issue in 67276712↗