Eggstravaganza
First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

A malicious user can steal someone else's Eggstravaganza NFT

juanalacubana

Summary

A malicious user can input their own address and the tokenId of someone else's Eggstravaganza NFT in the depositEgg function. Then when they call the withdrawEgg function they will pass the require check since they are both the msg.sender and depositor, allowing them to transfer anyone's Eggstravaganza NFT to their own wallet.

Vulnerability Details

// @notice Records the deposit of an egg (NFT).
// The NFT must already have been transferred to the vault.
function depositEgg(uint256 tokenId, address depositor) public {
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}
// @notice Allows the depositor to withdraw their egg from the vault.
function withdrawEgg(uint256 tokenId) public {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
​
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}

In EggVault.sol the function depositEgg takes in a parameter tokenId of type unit256 and a parameter depositor of type address. The problem is that anyone can put their own address in as the depositor and input the tokenId of someone else's Eggstravaganza NFT. Then this records the tokenId to the address of depositor. Then in the function withdrawEgg you can input the same tokenId and since now that the tokenId is set to the depositor, it'll just have a require statement to check and see if the msg.sender is the depositor. Which is true since the malicious user input their own address as the depositor and they are the one calling the withdrawEgg function. Which will then allow them to transfer the Eggstravaganza NFT from the EggVault.sol contract to their wallet.

Impact

Anyone can input the tokenId of someone else's Eggstravaganza NFT and take owner ship of the NFT over them and withdraw the token from the vault to their wallet.

Tools Used

Manual Review

Recommendations

In the depositEgg function remove the parameter for depositor of type address and include this code instead.

function depositEgg(uint256 tokenId) public {
// Check if the caller owns the NFT
require(eggNFT.ownerOf(tokenId) == msg.sender, "You do not own this NFT");
// Check if the NFT has already been deposited (prevent duplicate deposits)
require(!storedEggs[tokenId], "Egg already deposited");
​
// Transfer the NFT to the contract (Eggvault)
eggNFT.transferFrom(msg.sender, address(this), tokenId);
​
// Mark the egg as deposited and assign the depositor
storedEggs[tokenId] = true;
eggDepositors[tokenId] = msg.sender;
​
emit EggDeposited(msg.sender, tokenId);
}
​

This way we first check to see if the owner of the NFT is the msg.sender and if yes then we transfer the NFT from msg.sender to address(this) which is the EggVault.sol contract.

Updates

Lead Judging Commences

m3dython Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Frontrunning Vulnerability DepositEgg

Front-running depositEgg allows deposit ownership hijacking.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.