claim function: Excess ether did not return to the user

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/main/src/PhiFactory.sol#L264-L304

Vulnerability details

Impact

The claim function in PhiFactory contract is used to claim an art reward. The mint fee is calculated by the getArtMintFee function and will be sent to merkleClaim or signatureClaim:

 uint256 mintFee = getArtMintFee(artId, quantity_);
 this.merkleClaim{ value: mintFee }(proof, claimData, mintArgs, leafPart_);

uint256 mintFee = getArtMintFee(artId, quantity_);
this.signatureClaim{ value: mintFee }(signature_, claimData, mintArgs);

Users are likely to send more ether than the actual fee in order to ensure that they can claim the art. However, the claim function did not return the excess ether, which would cause asset loss to the user.

Proof of Concept

https://github.com/code-423n4/2024-08-phi/blob/main/src/PhiFactory.sol#L264-L304

Tools Used

Manual Review

Recommended Mitigation Steps

Consider following fix:

uint256 mintFee = getArtMintFee(artId, quantity_);
this.merkleClaim{ value: mintFee }(proof, claimData, mintArgs, leafPart_);
if (msg.value > mintFee){
     _msgSender().safeTransferETH(msg.value - mintFee);
}

uint256 mintFee = getArtMintFee(artId, quantity_);
this.signatureClaim{ value: mintFee }(signature_, claimData, mintArgs);
if (msg.value > mintFee){
    _msgSender().safeTransferETH(msg.value - mintFee);
}

Assessed type

ETH-Transfer

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 marked the issue as not a duplicate

[c4-judge]

fatherGoose1 marked the issue as duplicate of #109