It is a good idea to add a require() statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer()/safeTransferFrom() unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.
However, using require() to check transfer return values could lead to issues with non-compliant ERC20 tokens which do not return a boolean value. Therefore, it's highly advised to use OpenZeppelin’s safeTransfer()/safeTransferFrom().
Tools Used
Manual Review
Recommended Mitigation Steps
Consider using safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom().
Lines of code
https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/L1GraphTokenGateway.sol#L235 https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/L1GraphTokenGateway.sol#L276
Vulnerability details
Impact
It is a good idea to add a
require()statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’ssafeTransfer()/safeTransferFrom()unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.However, using
require()to check transfer return values could lead to issues with non-compliant ERC20 tokens which do not return a boolean value. Therefore, it's highly advised to use OpenZeppelin’ssafeTransfer()/safeTransferFrom().Tools Used
Manual Review
Recommended Mitigation Steps
Consider using
safeTransfer()/safeTransferFrom()instead oftransfer()/transferFrom().