Incorrect implementation of ERC-677

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/f86279e76fd9f810d2a25243012e1be4191a547e/contracts/ERC20.sol#L162

Vulnerability details

Impact

The implementation of ERC677 leads to the following issues:

• It is impossible to use transferAndCall to transfer tokens to a regular user.
• If the recipient returns false, which means they failed to process the token reception, the tokens will still end up on their contract since there is no check for the result of the onTokenTransfer call, which leads to a loss of the very essence of using this standard.

Proof of Concept

The full implementation of the contract is as follows:
https://github.com/code-423n4/2023-04-frankencoin/blob/f86279e76fd9f810d2a25243012e1be4191a547e/contracts/ERC20.sol#L162

File: contracts\ERC20.sol

162:     function transferAndCall(address recipient, uint256 amount, bytes calldata data) external override returns (bool) {
163:         bool success = transfer(recipient, amount);
164:         if (success){
165:             success = IERC677Receiver(recipient).onTokenTransfer(msg.sender, amount, data);
166:         }
167:         return success;
168:     }

Here we can see that there is no possibility of transferring tokens through this method to a regular user(since a simple user cannot handle the onTokenTransfer call), and the result of the 'onTokenTransfer' call is not checked, leading to a situation where tokens are sent in L163 but not accepted in L165, which may result in their being locked on the recipient's contract.

Links on the resource side:

• https://github.com/smartcontractkit/LinkToken/blob/master/contracts/v0.6/ERC677.sol
• ERC: transferAndCall Token Standard ethereum/EIPs#677

Tools Used

• Manual review
• Hardhat

Recommended Mitigation Steps

• Fix the implementation or replace it with the current actual standard

[c4-pre-sort]

0xA5DF marked the issue as low quality report

[0xA5DF]

Insignificant impact

[c4-judge]

hansfriese marked the issue as unsatisfactory:
Overinflated severity