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

`EggVault::depositEgg` is vulnerable to front-running due to lack of access control

0xkemah

Description: The EggVault::depositEgg function is publicly accessible, allowing any external caller to execute it. In a situation where a player does not call the intended EggHuntGame::depositEggToVault and manually transfers the EggstravaganzaNFT, an attacker can frontrun the subsequent EggVault::depositEgg call by passing the an arbitrary address as the depositor and illegitimately claim the unclaimed tokenId of the NFT. This is possible because there is no proper access control on the public EggVault::depositEgg function.

Impact: Unauthorized users might be able to claim ownership of NFTs sent to the vault by legitimate players, by frontrunning the EggVault::depositEgg transaction. This can lead to loss of assets as ownership can be stolen before rightful ownership is established, undermining trust in the protocol.

Proof of Code:

Code

function test_UserCanIllegitimatelyClaimOwnershipOfNft() public {
// alice mint an NFT and transfer it to the vault
vm.prank(address(game));
nft.mintEgg(alice, 10);
vm.prank(alice);
nft.transferFrom(alice, address(vault), 10);
​
// bob claims the NFT as the depositor
vm.prank(bob);
vault.depositEgg(10, bob);
​
// alice tries to claim the NFT as the legitimate depositor
vm.prank(alice);
vm.expectRevert("Egg already deposited");
vault.depositEgg(10, alice);
​
// bob withdraws the NFT
vm.prank(bob);
vault.withdrawEgg(10);
​
// check that bob is the owner of the NFT
assert(nft.ownerOf(10) == bob);
}

Recommended Mitigation: A strict access control mechanism should be implemented to ensure that only the EggHuntGame contract is allowed to call EggVault::depositEgg. Additionally, users should be explicitly warned not to send NFTs directly to the contract via transferFrom, as such transfers cannot be tracked or recovered by the contract, potentially resulting in asset loss.

+ address public gameContract;
+ constructor() Ownable(msg.sender) {}
​
+ function setGameContract(address _gameContract) external onlyOwner {
+ require(_gameContract != address(0), "Invalid game contract address");
+ gameContract = _gameContract;
+ }
​
function depositEgg(uint256 tokenId, address depositor) public {
+ require(msg.sender == gameContract);
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
...
}
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.