Some ERC20 tokens do not conform to the standard of returning a boolean when transfer is called. If one of these tokens is included as a reward token, the withdraw function will be irrevocably broken, and users won't be able to collect their reward or their deposit tokens. The transferFrom function may work fine, so adding the token works, but withdrawing breaks.
Proof of Concept
See, impact, which pretty much explains it.
Tools Used
Manual Analysis
Recommended Mitigation Steps
Use SafeERC20 library from OpenZeppelin for calling transfer.
Lines of code
https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L224-L236
Vulnerability details
Impact
Some ERC20 tokens do not conform to the standard of returning a boolean when
transfer
is called. If one of these tokens is included as a reward token, the withdraw function will be irrevocably broken, and users won't be able to collect their reward or their deposit tokens. ThetransferFrom
function may work fine, so adding the token works, but withdrawing breaks.Proof of Concept
See, impact, which pretty much explains it.
Tools Used
Manual Analysis
Recommended Mitigation Steps
Use SafeERC20 library from OpenZeppelin for calling transfer.