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

Reentrancy vulnerability in EggVault withdrawEgg function

teoslaf

Summary

The EggVault::withdrawEgg function modifies state variables after making an external call, violating the checks-effects-interactions pattern and creating a potential reentrancy vulnerability.

Vulnerability Details

The withdrawEgg function updates state variables before making an external call to transfer the NFT:

## EggVault.sol
​
function withdrawEgg(uint256 tokenId) public {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
​
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}

While direct reentrancy with the same token ID is prevented by the state changes, this pattern still violates best practices and could be vulnerable to cross-function reentrancy attacks.

Impact

  • Potential for cross-function reentrancy attacks

  • Could be exploited in combination with other contracts or protocols

  • Violates smart contract security best practices

The severity is medium rather than high because:

  • Direct reentrancy with the same token ID is prevented by state changes

  • The NFT contract uses _mint instead of _safeMint, limiting callback opportunities

  • The impact is constrained by the unique nature of NFTs

Tools Used

  • Manual code review

Recommendations

Implement a reentrancy guard and follow the checks-effects-interactions pattern:

+ import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
- contract EggVault is Ownable {
+ contract EggVault is Ownable, ReentrancyGuard {
​
// ... existing code ...
​
- function withdrawEgg(uint256 tokenId) public {
+ function withdrawEgg(uint256 tokenId) public nonReentrant {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
​
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}

This change adds the OpenZeppelin ReentrancyGuard and applies the nonReentrant modifier to the withdrawEgg function, preventing any reentrancy attacks.

Updates

Lead Judging Commences

m3dython Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.