Overpayment Due to Lack of Refund Mechanism in EntityForging::forgeWithListed: Financial Imbalance and User Distrust

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

In the EntityForging::forgeWithListed function, the protocol ensures that msg.value is at least equal to or greater than the forging fee. However, this check does not account for any excess funds paid by the player, leading to overpayments that are not refunded. This results in a gradual depletion of user funds without their awareness, affecting the game's economic balance.

Proof of Concept

this below lines shows you the particular validation in question
https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntityForging/EntityForging.sol#L126

Recommended Mitigation

proper accounting is very crucial to pay attention to, implementation of a validation that calculates and send back the remaining back to the player.

• An example of such code can be such in TraitForgeNft::mintToken below you will see how excess is been accounted for

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.");
        }
    }

Assessed type

Other

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[c4-judge]

koolexcrypto marked the issue as grade-c

[c4-judge]

This previously downgraded issue has been upgraded by koolexcrypto

[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)