code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

Calls transfer()/transferFrom() With IERC20 #4

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L84

Vulnerability details

Calls transfer()/transferFrom() With IERC20

Description

The transfer() and transferFrom() methods are part of the ERC-20 standard, designed for transferring tokens between accounts. However, these methods could be risky when transferring tokens to smart contracts because they don't guarantee that the receiving smart contract will handle the transferred tokens correctly, potentially leading to permanent token loss. ERC-20 standard doesn't include a 'onTokenReceived' function or similar to alert the recipient about incoming transfers. Therefore, the 'transfer' and 'transferFrom' functions are not reliable for interactions with arbitrary contracts. A safer alternative is to use safeTransfer() and safeTransferFrom() methods provided by libraries like OpenZeppelin's SafeERC20. These methods add a layer of protection by checking if the recipient is a contract and if it has a function to handle incoming tokens. If these checks are not passed, the transfer operation is not executed, hence preventing potential token loss.

There are 4 instances of this issue: ### - File: contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol ``` Line: 84 transfer(from, to, amount, true, "") ``` use safeTransferFrom instead. [https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L84](https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L84) - File: contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol ``` Line: 99 transfer(msg.sender, to, amount, true, "") ``` use safeTransferFrom instead. [https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L99](https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20.sol#L99) - File: contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol ``` Line: 92 transfer(from, to, amount, true, "") ``` use safeTransferFrom instead. [https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol#L92](https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol#L92) - File: contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol ``` Line: 107 transfer(msg.sender, to, amount, true, "") ``` use safeTransferFrom instead. [https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol#L107](https://github.com/code-423n4/2023-06-lukso/blob/main/contracts/LSP7DigitalAsset/extensions/LSP7CompatibleERC20InitAbstract.sol#L107)

Assessed type

ERC20

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

minhquanym commented 1 year ago

Seems invalid

trust1995 commented 1 year ago

Conditionals on user error are not accepted as M severity.

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid