Eggstravaganza

First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Invalid

The owner can make Eggs inaccessible to players who deposit Eggs in the vault.

Summary

When the EggVault contract is deployed, the owner must set the address of the NFT contract whose NFTs can be deposited into the vault. The owner does this using the EggVault::setEggNft function, which can be called as many times as the owner desires.

Vulnerability Details

A malicious owner could exploit this after several players have deposited their NFT Eggs into the vault. By calling EggVault::setEggNft, the owner can change the NFT contract address. As a result, the transferFrom function in EggVault::withdrawEgg would no longer interact with the original NFT contract where the players’ NFTs are stored, preventing players from withdrawing their deposited NFT Eggs.

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);
}

Impact

A player who deposits their Eggs into the vault could find themselves in a situation where the owner changes the NFT contract address, rendering them unable to withdraw their NFTs.

Tools Used

  • VS code: Cloned the repository locally and identified the vulnerability through manual review.

Recommendations

The address of the EggVault::eggNFT contract could be defined in the constructor and marked as immutable to prevent it from being changed. This would ensure players can trust that, if the address is correct at deployment, they won’t end up in a situation where they cannot withdraw their NFT Eggs. Function EggVault::setEggNft can be removed then.

-EggstravaganzaNFT public eggNFT;
+EggstravaganzaNFT public immutable eggNFT;
...
-constructor()Ownable(msg.sender, ){
+constructor(address _eggNFTAddress)Ownable(msg.sender, ){
+ eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
-/// @notice Set the NFT contract address.
-function setEggNFT(address \_eggNFTAddress) external onlyOwner {
- require(\_eggNFTAddress != address(0), "Invalid NFT address");
- eggNFT = EggstravaganzaNFT(\_eggNFTAddress);
-}
Updates

Lead Judging Commences

m3dython Lead Judge 2 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Trusted Owner

Owner is trusted and is not expected to interact in ways that would compromise security

Support

FAQs

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