howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntityForging/EntityForging.sol#L126

Vulnerability details

Impact

The function forgeWithListed has a check that may cause potential loss for Nft owners.

Proof of Concept

In EntityForging.sol contract forgeWithListedfunction can be called by merger owners so that they can forge their Nfts with forger tokens. The following check:

require(msg.value >= forgingFee, 'Insufficient fee for forging');

ensures that the value provided by the merger owner is greater and equal to the fee set by forger owner.However if merger owner unintentionally (a typo) provide greater than forgingFee, there is no code that refunds the excess amount to the user. Therefore this check is an open door to a possible vulnerability that result in potential loss for merger owners.

Tools Used

Manual Review, Vs Code

Recommended Mitigation Steps

There are 2 possible mitigations: First implement exact amount with the fee. As it is done in EntityTrade contract buyNft Second adjust the code to prevent any potential loss due to overpayment.

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

    (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');

   // Refund any excess amount to the merger owner
+++    if (msg.value > forgingFee) {     (bool refundSuccess, ) = msg.sender.call{ value: msg.value - forgingFee }('');
        require(refundSuccess, 'Failed to refund excess amount');
    }

    // Cancel listed forger nft
    _cancelListingForForging(forgerTokenId);
     ...
   //SKIPPED
  }

Assessed type

Invalid Validation