howlbot-integration[bot] commented 2 months ago

Lines of code

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

Vulnerability details

Impact

In the function EntityForging::forgeWithListed, there is a check msg.value >= forgingFee. However, if msg.value > forgingFee, the excess funds are not utilized and remain locked in the EntityForging contract permanently. This issue can lead to a loss of user funds that cannot be recovered or used by any party.

I raise this issue as Medium for the reason that funds are permanently locked and can never be retrieved by any party, which is beyond the severity of QA.

Proof of Concept

In the forgeWithListed function, the amount sent with the transaction (msg.value) is checked against the required forging fee:

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

If msg.value exceeds forgingFee, the excess amount is not handled and remains locked in the contract:

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

There is no mechanism to refund the excess amount to the sender or allocate it elsewhere. This results in the excess funds being permanently locked in the contract.

Note: I raise this issue as Medium for the reason that funds are permanently locked and can never be retrieved by any party.

Tools Used

Manual

Recommended Mitigation Steps

To mitigate this issue, refund the sender with the excess funds or allocate them to an appropriate recipient such as the forge owner or contract owner.

Assessed type

Payable