There are several ERC20 tokens that do not revert on failure. Therefore, it is possible for a function call containing transfer() or transferFrom() to succeed even if the ERC20 token transfer did not. This can lead to loss of funds for Rubicon users, specifically in the BathToken._withdraw() function. In the case of an ERC20 that does not revert on failure, if the contract does not have enough tokens to cover the transfer, the user will simply not receive any value from their withdrawal and their bath tokens will still be burnt.
bghughes
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L605 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L194-L215
Vulnerability details
Impact
There are several ERC20 tokens that do not revert on failure. Therefore, it is possible for a function call containing
transfer()ortransferFrom()to succeed even if the ERC20 token transfer did not. This can lead to loss of funds for Rubicon users, specifically in theBathToken._withdraw()function. In the case of an ERC20 that does not revert on failure, if the contract does not have enough tokens to cover the transfer, the user will simply not receive any value from their withdrawal and their bath tokens will still be burnt.Tools Used
Manual review
Recommended Mitigation Steps
Always use
safeTransfer()andsafeTransferFrom().