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

3 stars 1 forks source link

Unrestricted Name and Symbol Modification in LSP7 and LSP8 Digital Assets #93

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/LSP7DigitalAsset.sol#L28-L29 https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.sol#L26-L27

Vulnerability details

I HAVE ALREADY SUBMITTED THIS ISSUE HOWEVER I MESSED UP THE LINKS FOR IT. CAN YOU PLEASE DISREGARD THE PREVIOUS SUBMISSION?

Impact

The owner of a contract in LSP8IdentifiableDigitalAsset and LSP7DigitalAsset can arbitrarily change the name and symbol of a token after its deployment. This ability is due to the inheritance of the setData() function from ERC725YCore.sol implemented in LSP4DigitalAssetMetadata.

Such functionality poses a significant risk as these attributes can potentially influence the perceived value and trust of the token in the market. A sudden or unanticipated change in these attributes, like setting them to empty strings, may lead to confusion, decreased trust, and impact the intrinsic value of the token. Therefore, it could lead to market manipulations and potential financial loss for token holders.

Proof of Concept

The issue is found in the ERC725YCore code, specifically in lines 62-68, where any data key can be overridden. This includes the name and symbol of the tokens under LSP7 and LSP8 contracts.

For direct code references, please see: ERC725YCore.sol#L62-L68

Tools Used

The code was reviewed manually without the use of specific security tools.

Recommended Mitigation Steps

It is advisable to limit the ability to modify the token's name and symbol after the contract deployment. This can be achieved by:

Setting the name and symbol attributes as immutable. Once assigned during contract deployment, these variables cannot be changed.

Overriding the setData() function to disallow the modification of specific keys related to the token's name and symbol. This requires careful implementation to avoid unintentionally restricting modification of other necessary keys.

In general, any function or action that can influence the token's characteristics, perceived value, or operation should have appropriate access controls to prevent misuse and maintain the token's integrity.

Assessed type

Other

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #91

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid