code-423n4 / 2024-07-traitforge-findings

2 stars 1 forks source link

Funds can be locked indefinitely in NukeFund.sol #1078

Open howlbot-integration[bot] opened 2 months ago

howlbot-integration[bot] commented 2 months ago

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

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:

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.

  1. 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

OR

 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

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 commented 2 months ago

koolexcrypto changed the severity to 2 (Med Risk)

c4-judge commented 2 months ago

koolexcrypto marked the issue as primary issue

koolexcrypto commented 2 months ago

For context, 594

c4-judge commented 2 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 2 months ago

koolexcrypto marked the issue as selected for report