Inheritable Smart Contract Wallet

Summary

The NFTFactory contract’s functions createEstate and burnEstate are restricted to calls from the designated inheritanceManager. However, if the inheritanceManager is itself a contract with malicious intent, it could exploit the fact that external calls (such as _mint and _burn) are performed without a reentrancy guard or proper state updating beforehand. In such a scenario, a reentrant call could be triggered during the minting process, leading to the recursive creation of NFTs and potential uncontrolled inflation before state changes (e.g., the counter increment) are fully finalized.

Vulnerability Details

• Issue:
  The functions createEstate and burnEstate make external calls via _mint and _burn (inherited from OpenZeppelin’s ERC721 implementation) while relying solely on the onlyInheritanceManager modifier for access control. If the inheritanceManager is implemented as a contract that can perform external calls (for example, in its fallback function), it may reenter createEstate before the counter is properly updated. This reentrancy vulnerability allows the malicious manager to mint an arbitrary number of NFTs.

• Root Cause:
  The core issue is that the contract does not employ a reentrancy guard or adopt the Checks-Effects-Interactions pattern. State variables (like the NFT counter) are updated after the external call (_mint), allowing a malicious inheritanceManager to exploit the window of vulnerability to recursively call createEstate.

• Impact:
  compromised inheritanceManager could recursively call createEstate to mint an unlimited number of NFTs before the counter is fully updated.

• Tools Used:

  • Foundry/Forge:

  • Manual code review and simulation tests using Foundry.

Mitigation

To remediate this vulnerability, implement the following fixes:

1. Apply a Reentrancy Guard:

   • Import OpenZeppelin’s ReentrancyGuard and add the nonReentrant modifier to functions like createEstate and burnEstate.

   import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

   ​

   contract NFTFactory is ERC721URIStorage, ReentrancyGuard {

   // ...

   function createEstate(string memory description) external onlyInheritanceManager nonReentrant returns (uint256 itemID) {

   uint256 ID = _incrementCounter();

   _mint(msg.sender, ID);

   _setTokenURI(ID, description);

   return ID;

   }

   }

2. Adopt the Checks-Effects-Interactions Pattern:

   • Update state variables (e.g., the NFT counter) before performing external calls such as _mint. This minimizes the window for reentrant calls.

   • For example:

   function createEstate(string memory description) external onlyInheritanceManager nonReentrant returns (uint256 itemID) {

   uint256 ID = _incrementCounter(); // State updated first

   // External call follows

   _mint(msg.sender, ID);

   _setTokenURI(ID, description);

   return ID;

   }