Submission Details
Severity:
An attacker can steal the token of another player (because `EggVault::depositEgg` requires the token to be owned by the vault) if the player choses to deposit his token without using the `EggHuntGame::depositEggToVault` function
### [H-2] An attacker can steal the token of another player (because `EggVault::depositEgg` requires the token to be owned by the vault) if the player choses to deposit his token without using the `EggHuntGame::depositEggToVault` function
**Description:** `EggVault::depositEgg` requires that the token is first transfered to the `EggVault` contract. If a player wants to deposit his token wihtout using the function `EggHuntGame::depositEggToVault` he must first transfer ownership of the token to the vault and then call the function `depositEgg` leaving a window for an attacker to claim the token
**Impact:** Any player who choses to not use the `EggHuntGame::depositEggToVault` to deposit their token into the vault is susceptible to losing their token if an attacker calls the function `EggVault::depositEgg` after the player transfered the ownership and before he could call `EggVault::depositEgg` himself
**Proof of Concept:**
1. Player finds the eggNFT
2. Player transfers the ownership of the eggNFT to the vault to be later deposited
3. Attacker calls the function `EggVault::depositEgg` before the player could
4. The token is deposited with the value of the depositor equals the address of the attacker
5. Player gets his transaction reverted because the token is already assigned to the attacker
```javascript
function testStealToken() public {
address player = address(0x123);
address attacker = address(0x1232);
vm.prank(address(game));
nft.mintEgg(player,20);
vm.prank(player);
nft.transferFrom(player,address(vault),20);
vm.prank(attacker);
vault.depositEgg(20,attacker);
vm.expectRevert();
vm.prank(player);
vault.depositEgg(20,player);
}
```
**Recommended Mitigation:** Modify the `EggVault::depositEgg` to require that the msg.sender is the initial owner of the token not the vault.
```diff
- function depositEgg(uint256 tokenId, address depositor) public {
+ function depositEgg(uint256 tokenId) public {
- require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
+ require(eggNFT.ownerOf(tokenId) == msg.sender, "NFT not owned by the sender");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
- eggDepositors[tokenId] = depositor;
+ eggDepositors[tokenId] = msg.sender;
- emit EggDeposited(depositor, tokenId);
+ emit EggDeposited(msg.sender, tokenId);
}
```
Updates
Lead Judging Commences
m3dython 4 months ago
Submission Judgement Published Assigned finding tags:
Frontrunning Vulnerability DepositEgg
Front-running depositEgg allows deposit ownership hijacking.
Rewards breakdown
nSLOC
129This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.