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

3 stars 1 forks source link

Potential Reversion in Transfer due to LSP1 Interface Support Check #94

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP7DigitalAsset/LSP7DigitalAssetCore.sol#L452-L467 https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAssetCore.sol#L454-L469

Vulnerability details

Impact

The transfer function in LSP7DigitalAssetCore & LSP8DigitalAssetCore includes a mandatory hook, _notifyTokenSender, which verifies if the sender supports _INTERFACEID_LSP1. However, if a token owner who initially implemented LSP1 interface ceases to support it, this verification fails, causing an error in the transfer function.

This is a significant issue as it can interrupt token transfers in situations like DeFi protocols, where a user might have granted permissions to a swapping pool. For example, an old UniversalProfile (UP) owner could have set a Universal Receiver Delegate under _LSP1_UNIVERSAL_RECEIVER_DELEGATE_KEY, which tries to call back setData on the UP. If the UP's new owner is an Externally Owned Account (EOA) that has not modified all keys of the old owner, this callback would fail, causing the transfer function to revert, disrupting swaps or other operations.

Proof of Concept

This issue could occur when a UP changes its ownership from a contract (e.g., LSP6KeyManager) to an EOA. In such a case, the old Universal Receiver Delegate would attempt to call setData on the UP during the execution of the _notifyTokenSender hook, which would fail because EOAs don't support it (see LSP20). This would cause a reversion in the transfer function, disrupting operations.

Tools Used

Recommended Mitigation Steps

Assessed type

DoS

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

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 duplicate of #94

c4-judge commented 1 year ago

trust1995 marked the issue as not a duplicate

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid