NFT Reported Errors

[gagdiez]

Background

A user has reported the following errors:

1. We noticed that in the Series contract, create_series function creates a storage key for tokens by
appending given series id with account id of the caller. However, there is no separator between them. It leads to the key collision 

This combination can lead to the storage key collision:
series_id = 2
account_id = 2clashing.test.near
storage_key = sha256(22clashing.test.near)
series_id_2 = 22
account_id_2 = clashing.test.near
storage_key_2 = sha256(22clashing.test.near)
storage_key_2 == storage_key


https://github.com/near-examples/nft-tutorial/blob/main/nft-series/src/series.rs#L39 

On each token mint, the storage will be overwritten for both of the series (2 & 22), which will lead to the series containing tokens from other series. Any series creator can exploit this by setting a series id to match the storage location of the token set of another series. It can be done for simple griefing, or if they match the token storage location of, let's say, an "expensive" series; when mint of an expensive series token happens, cheap series will also have that token in their set. Then, depending on the logic of the application, it can be exploited further.


Let me know if you need POC


2. We noticed that if the series contains a large number of royalties, during the nft_transfer_payout
execution, the contract can panic due to underflow on royalty_to_payout. This leads to some of the
tokens to be unpayable. That's because there is no validation of whether the total royalty percentage is larger than %100. So, a series creator can create a series with an arbitrary total percentage of royalties to be paid

https://github.com/near-examples/nft-tutorial/blob/main/nft-series/src/royalty.rs#L161
https://github.com/near-examples/nft-tutorial/blob/main/nft-series/src/series.rs#L15

Acceptance Criteria

• There is no more collision on series
• The royalties are validated

Priority

• 🟠 P1 : High