Lines of code

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

Vulnerability details

Impact

Forging with a listed item incurs an excessive cost.

Proof of Concept

Function forgeWithListed doesn't refund the excess amount (msg.value - forgingFee) to the caller, resulting in users overpaying for the forging process.

  function forgeWithListed(
    uint256 forgerTokenId,
    uint256 mergerTokenId
  ) external payable whenNotPaused nonReentrant returns (uint256) {
    Listing memory _forgerListingInfo = listings[listedTokenIds[forgerTokenId]];
    [...]

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

    [...]

    uint256 devFee = forgingFee / taxCut;
    uint256 forgerShare = forgingFee - devFee;

    [...]

    return newTokenId;
  }

Tools Used

Manual review

Recommended Mitigation Steps

Refund the remaining amount after deducting the forging fee to ensure users are not overcharged.

Code Correction Add the refund logic at the end of the forgeWithListed function:


function forgeWithListed(  
    uint256 forgerTokenId,  
    uint256 mergerTokenId  
) external payable whenNotPaused nonReentrant returns (uint256) {  
    Listing memory _forgerListingInfo = listings[listedTokenIds[forgerTokenId]];  
    [...]  

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

    [...]  

    uint256 devFee = forgingFee / taxCut;  
    uint256 forgerShare = forgingFee - devFee;  

    [...]  

+    // Refund excess amount to the caller  
+    uint256 excessPayment = msg.value - forgingFee;  
+    if (excessPayment > 0) {  
+        (bool refundSuccess, ) = msg.sender.call{ value: excessPayment }('');  
+        require(refundSuccess, 'Refund failed.');  
+    }  

    return newTokenId;  
}  

## Assessed type

Token-Transfer

code-423n4 / 2024-07-traitforge-validation