DittoETH

Summary

Right now the yield claiming responsibility before a NFT transfer lies with the user. This will inevitably lead to users losing funds.

Vulnerability Details

Right now the NFT (which corresponds to a ShortRecord) transfer works as follows:

1. Monica mints an NFT that corresponds to a shortRecord id that she owns.

2. She approves the transfer to Ross.

3. She calls transferForm to transfer her NFT to Ross.

4. Within the transferFrom function in ERC721Facet.sol the function LibShortRecord.transferShortRecord is called.

5. The transferShortRecord function calls deleteShortRecord and then calls createShortRecord to create a new ShortRecord for the receipient.

6. The deleteShortRecord function wipes the initial owner's yield.

The risk of getting a legitimate user's yield deleted is unnecessary and can be easily mitigated. Users will not know their yield is about to be deleted, or if they know they will most likely forget.

Impact

Unnecessary and easily mitigatable loss of users funds.

Tools Used

Manual review

Recommendations

Introduce parts of the distribution logic into the transferShortRecord from LibShortRecord.sol in order to attempt the distribution yield for the from address.