In this case, since GRT token is used, the current implementation of GRT does have a return value for transferFrom and reverts on failure, but the same cannot be said for many other ERC20 tokens in the wild. OpenZeppelin recommends to always use safeTransferFrom whenever possible, and it would be better to err on the side of caution.
Lines of code
https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L235 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L276
Vulnerability details
Impact
In this case, since GRT token is used, the current implementation of GRT does have a return value for
transferFrom
and reverts on failure, but the same cannot be said for many other ERC20 tokens in the wild. OpenZeppelin recommends to always use safeTransferFrom whenever possible, and it would be better to err on the side of caution.Proof of Concept
Two instances of the use of
transferFrom
can be found in L1GraphTokenGateway.sol#L235 and L1GraphTokenGateway.sol#L276Recommended Mitigation Steps
Use
IERC20(token).safeTransferFrom()
instead.