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
If the underlying ERC20 token does not revert on failure, the protocol's internal accounting will record failed transfer operations as successful.
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
This issue has been acknowledged by Wasabi, and a fix was implemented in commit 0b7bffe6↗.