Excess payment in EntityForging::forgeWithListed will be stuck in contract, leading to loss of players' funds

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

In the EntityForging::forgeWithListed function, any excess amount of msg.value over the required forgingFee is not refunded and will be stuck in the contract, leading to loss of players' funds. The contract currently only requires that msg.value be greater than or equal to the forgingFee, but there is no logic to refund any excess amounts sent.

Proof of Concept

The aforesaid check in EntityForging::forgeWithListed is extracted below:

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

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

There is no logic to refund any excess amounts sent. Note that such a refund logic is present in the mint functions of TraitForgeNft.sol. One example is shown below:

function mintToken(
    bytes32[] calldata proof
  )
    public
    payable
    whenNotPaused
    nonReentrant
    onlyWhitelisted(proof, keccak256(abi.encodePacked(msg.sender)))
  {
    uint256 mintPrice = calculateMintPrice();
    require(msg.value >= mintPrice, 'Insufficient ETH send for minting.');

    _mintInternal(msg.sender, mintPrice);

    uint256 excessPayment = msg.value - mintPrice;
@>    if (excessPayment > 0) {
      (bool refundSuccess, ) = msg.sender.call{ value: excessPayment }('');
      require(refundSuccess, 'Refund of excess payment failed.');
    }
  }

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

Tools Used

Manual review

Recommended Mitigation Steps

Implement a refund logic in EntityForging::forgeWithListed for excess amounts sent, similar to that in the mint functions of TraitForgeNft.sol

Assessed type

Other

[c4-judge]

koolexcrypto marked the issue as grade-c

[c4-judge]

koolexcrypto marked the issue as duplicate of #687

[c4-judge]

koolexcrypto marked the issue as duplicate of #218

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)