Eggstravaganza

HIGH

[H-1] Weak and Predictable Randomness in EggHuntGame.sol::searchForEgg which can be manipulated by miners or influenced by users

Description: Hashing block.timestamp, block.prevrandao, and msg.sender for randomness, which can be manipulated by miners or influenced(predict) by the user. The current block timestamp is publicly accessible and can be influenced within a certain range by miners or validators. block.prevrandao can still be manipulated to some extent by validators, making it less secure for generating random values.

Impact: any user or miner can influence and manipulate the randomn number, thereby inreasing there eggs found count anytime the execute the searchegg function to their advantage.

Proof of Concept:

1. An attacker observing the mempool can predict the outcome of the random number generation before the transaction is mined. If the outcome is unfavorable, they can choose not to execute the transaction, or they can manipulate transaction parameters to achieve a desired outcome.

2. Validators have the capability to adjust certain block parameters, such as the timestamp, within a permissible range. By doing so, they can influence the random number generation to their advantage. ​

Recommended Mitigation:

Integrate with external randomness oracles like Chainlink VRF, which provide verifiable and tamper-proof random values.

MEDIUM

[M-1] Using transferFrom to send Nft to users in EggVault::WithdrawEgg function

Description: the EggVault::WithdrawEgg uses transferFrom to send nft to users. However, transferFrom doesn’t check if the receiver is a contract and can handle NFTs.

Impact: Tokens can get locked in contracts that don’t support ERC721Receiver, effectively bricking the token.

Proof of Concept:

1. user that finds an egg with a contract that doesn't support ERC721, the token can get locked, therefore the token cannot be transferred to the vault.

Recommended Mitigation: Prefer safeTransferFrom which verifies the receiver implements onERC721Received.

[M-2] Using ERC721::_mint()can be dangerous

Description: the function EggHuntGame::searchForEgg mints nft to a user that has a random number less than eggFindThreshold,ERC721::_mint() mints tokens to address without checking if it supports ERC721

Impact: Using ERC721::_mint() can mint ERC721 tokens to addresses which don't support ERC721 tokens.

Recommended Mitigation: Use _safeMint() instead of _mint() for ERC721

LOW

[L-1] No Event Emission in EggHuntGame::depositEggToVault

Description: the EggHuntGame::depositEggToVault is wrapping NFT transfers inside another function and don’t emit an events, tracking transfers becomes harder.

Impact: Poor traceability or missing audit logs.

Recommended Mitigation: Emit custom events for wrapped transfers. Emit events after significant actions, such as NFT transfers, can enhance transparency and facilitate off-chain tracking.

[L-2] Solidity Pragma should be specific not wide

Description the solidity pragma used on the contract should be specific not wide, Locking the pragma to a specific version ensures that the contract's behavior remains consistent across different compilations.​

impact

1. Solidity updates may introduce breaking changes or alter existing functionalities.

2. Specifying an exact version safeguards the contract from compiling with a version that might have incompatible changes.

Recommended Mitigation: Consider using a specific version of Solidity in your contracts instead of a wide version, instead of pragma solidity ^0.8.23;, use pragma solidity 0.8.23;