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 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!