Funds can be locked indefinitely in NukeFund.sol

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/NukeFund/NukeFund.sol#L40-L61
https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/NukeFund/NukeFund.sol#L153-L183

Vulnerability details

Impact

• Permanent loss of funds
• Dependency on user action

Proof of Concept

The NukeFund contract has a unique fund distribution model where the primary - and only - method to withdraw funds is through the nuke() function. This design creates a specific set of circumstances under which funds can be extracted from the contract.

Practical example:
Let's say the contract has accumulated 100 ETH in its fund. A user holding a qualified NFT (let's call it TokenID 42) wants to claim a portion of this fund.

function nuke(uint256 tokenId) public whenNotPaused nonReentrant {
    // ... (verification checks)
    uint256 claimAmount = /* calculated based on nuke factor */;
    fund -= claimAmount;
    nftContract.burn(tokenId);
    payable(msg.sender).call{value: claimAmount}("");
    // ... (event emissions)
}

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/NukeFund/NukeFund.sol#L153-L183

In this scenario:

• The user calls nuke(42).
• The function calculates the claim amount based on the token's properties.
• The NFT (TokenID 42) is burned.
• The calculated amount is sent to the user.
• The fund is reduced by the claimed amount.

This process works as intended, but it relies entirely on NFT holders initiating the withdrawal process. There's no other mechanism for fund distribution or withdrawal, which leads to the second point.

2. Risk of permanently locked funds

If all NFTs are burned or if no one calls the nuke() function, there's a real risk that funds could remain permanently locked in the contract.

Practical example:
Imagine a scenario where the contract has accumulated 500 ETH over time. There are only 10 NFTs left that are eligible for nuking.

Scenario A: All NFTs are burned

• Users with the remaining 10 NFTs all call nuke().
• After these transactions, all NFTs are burned.
• The contract still holds, say, 100 ETH (depending on the nuke factor calculations).
• There are no more NFTs left to trigger the nuke() function.
• The remaining 100 ETH is now locked in the contract with no way to withdraw it.

OR

• Users decide to directly call burn() to ball their NFTs

 function burn(uint256 tokenId) external whenNotPaused nonReentrant {
    require(
      isApprovedOrOwner(msg.sender, tokenId),
      'ERC721: caller is not token owner or approved'
    );
    if (!airdropContract.airdropStarted()) {
      uint256 entropy = getTokenEntropy(tokenId);
      airdropContract.subUserAmount(initialOwners[tokenId], entropy);
    }
    _burn(tokenId);
  }

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/TraitForgeNft/TraitForgeNft.sol#L141-L151

Scenario B: Inactivity

• Out of the 10 NFT holders, only 5 decide to call nuke().
• The other 5 forget about their NFTs or lose access to their wallets.
• The contract is left with 200 ETH and 5 unclaimed NFTs.
• If these 5 NFTs are never used to call nuke(), the 200 ETH remains locked indefinitely.

Tools Used

Manual review

Recommended Mitigation Steps

Add a function that allows the contract owner to withdraw funds in case of emergencies or when all NFTs are burned.

Assessed type

Context

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto marked the issue as primary issue

[koolexcrypto]

For context, 594

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report