Summary

The depositEgg function in the contract is used to record the deposit of an NFT (called an egg) into a vault. It checks that the vault owns the NFT and that the NFT has not already been deposited. However, there are some issues with how this function is designed.

Vulnerability Details

No Access Control

The function is marked as public, which means anyone can call depositEgg, even if they are not the person who transferred the NFT to the vault. This opens up the possibility for abuse. For example, a person could monitor transactions to see when an NFT is being sent to the vault and then call the function to deposit the NFT before the actual owner does. This is known as "front-running" and can cause problems, such as giving the wrong person credit for the deposit or messing up the vault’s logic.

No Events for Failed Deposits

Another issue is that the contract does not emit events when a deposit fails. For example, if the NFT is not in the vault or if it’s already deposited, the function won’t notify anyone. This makes it harder to track issues or catch bad actions during testing or when the contract is live. Adding events to show when a deposit fails can help monitor and debug the system.

Impact

The lack of access control is a significant issue. It could allow someone to falsely claim an NFT deposit as their own. This could lead to problems like stealing rewards, getting credit for ownership they don't deserve, or blocking the actual depositor from accessing their NFT.

Tools Used

Manual code review

Recommendations

To fix these problems, the depositEgg function should be updated to ensure only the person who actually owns the NFT can deposit it.

One way to do this is to use msg.sender (the address that calls the function) as the depositor instead of relying on an external argument. This ensures only the correct person can deposit their NFT.