Recipients of native funds can DOS users

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntityTrading/EntityTrading.sol#L63 https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntityForging/EntityForging.sol#L102 https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/NukeFund/NukeFund.sol#L40

Vulnerability details

Impact

The receiver of native funds can DOS the caller of multiple protocol functions.

Proof of Concept

On multiple places throughout the codebase, the push pattern is used for the transferring of native currency assets. This could be problematic if the receiver of the currency is not trusted because he can decide to make the call to revert thus DOS-ing the sender.

This could be problematic in the following functions:

• EntityTrading.buyNFT()
• EntityForging.forgeWithListed()
• NukeFund receive function sending native currency to the devAddress

buyNFT

Reference in code: link

  function buyNFT(uint256 tokenId) external payable whenNotPaused nonReentrant {
    Listing memory listing = listings[listedTokenIds[tokenId]];
    require(
      msg.value == listing.price,
      'ETH sent does not match the listing price'
    );
    require(listing.seller != address(0), 'NFT is not listed for sale.');

    //transfer eth to seller (distribute to nukefund)
    uint256 nukeFundContribution = msg.value / taxCut;
    uint256 sellerProceeds = msg.value - nukeFundContribution;
    transferToNukeFund(nukeFundContribution); // transfer contribution to nukeFund

    // transfer NFT from contract to buyer
@>  (bool success, ) = payable(listing.seller).call{ value: sellerProceeds }(
      ''
    );
    require(success, 'Failed to send to seller');
    nftContract.transferFrom(address(this), msg.sender, tokenId); // transfer NFT to the buyer

    delete listings[listedTokenIds[tokenId]]; // remove listing

    emit NFTSold(
      tokenId,
      listing.seller,
      msg.sender,
      msg.value,
      nukeFundContribution
    ); // emit an event for the sale
  }

1. Bob lists his NFT for sale on an attractive price.
2. Alice sees the listing and tries to buy it.
3. When the call tries to send native currency to Bob, the call reverts because Bob made his receive function to revert.
4. This way Bob can DOS buyers whenever he wants.

forgeWithListed

Reference in code: link

  function forgeWithListed(
    uint256 forgerTokenId,
    uint256 mergerTokenId
  ) external payable whenNotPaused nonReentrant returns (uint256) {
      ...

    require(
@>    nftContract.ownerOf(mergerTokenId) == msg.sender,
      'Caller must own the merger token'
    );
    require(
      nftContract.ownerOf(forgerTokenId) != msg.sender,
      'Caller should be different from forger token owner'
    );

    ...

    address payable forgerOwner = payable(nftContract.ownerOf(forgerTokenId));

    uint256 newTokenId = nftContract.forge(
      msg.sender,
      forgerTokenId,
      mergerTokenId,
      ''
    );
    (bool success, ) = nukeFundAddress.call{ value: devFee }('');
    require(success, 'Failed to send to NukeFund');
@>  (bool success_forge, ) = forgerOwner.call{ value: forgerShare }('');
    require(success_forge, 'Failed to send to Forge Owner');

        ...
  }

Here we have similar situation as with buyNFT().

1. Bob lists his NFT for forging.
2. Alice sees the listing and decides to forge and calls forgeWithListed().
3. Bob decides to DOS Alice once the native currency funds activate Bob’s receive function.

NukeFund receive

Reference in code: link

  receive() external payable {
    uint256 devShare = msg.value / taxCut; // Calculate developer's share (10%)
    uint256 remainingFund = msg.value - devShare; // Calculate remaining funds to add to the fund

    fund += remainingFund; // Update the fund balance

    if (!airdropContract.airdropStarted()) {
@>    (bool success, ) = devAddress.call{ value: devShare }('');
      require(success, 'ETH send failed');
      emit DevShareDistributed(devShare);
    } else if (!airdropContract.daoFundAllowed()) {
      (bool success, ) = payable(owner()).call{ value: devShare }('');
      require(success, 'ETH send failed');
    } else {
      (bool success, ) = daoAddress.call{ value: devShare }('');
      require(success, 'ETH send failed');
      emit DevShareDistributed(devShare);
    }

    emit FundReceived(msg.sender, msg.value); // Log the received funds
    emit FundBalanceUpdated(fund); // Update the fund balance
  }

When the airdrop is not started the funds that are coming from … will be sent to the devAddress. According to the readme the owner of the protocol is the only trusted entity so the devAddress could decide to revert the transaction.

This could DOS the following functions:

• forgeWithListed()
• buyNFT()
• mintToken()
• mintWithBudget()

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing a pull pattern (storing the owed amount in a mapping and having a function for the intended receiver of the funds to be able to withdraw the amount) when the receiver of the native currency is not trusted and this could alter the experience of other users (DOS).

Assessed type

DoS

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Insufficient quality