The classic openzepplin implementation of the ERC20 standard (ie. functions : Approve(), Transfer(), TransferFrom() and mint()) does not capture the fact that some ERC20 token do not return a boolean value (eg. BNB, USDT, OMG). As results, these functions won't work with these tokens.
Proof of Concept
see line of code above.
Tools Used
Manual audit
Recommended Mitigation Steps
Use SafeERC20 by openzeppling:
using SafeERC20 for ERC20;
then calling :
token.safeTransferFrom(…), etc.
Lines of code
https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/BridgeEscrow.sol#L29 https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/GraphTokenUpgradeable.sol#L98 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 https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/L2GraphToken.sol#L81
Vulnerability details
Impact
The classic openzepplin implementation of the ERC20 standard (ie. functions : Approve(), Transfer(), TransferFrom() and mint()) does not capture the fact that some ERC20 token do not return a boolean value (eg. BNB, USDT, OMG). As results, these functions won't work with these tokens.
Proof of Concept
see line of code above.
Tools Used
Manual audit
Recommended Mitigation Steps
Use SafeERC20 by openzeppling:
using SafeERC20 for ERC20;
then calling : token.safeTransferFrom(…), etc.