sherlock-audit / 2022-10-nftport-judging

1 stars 0 forks source link

GimelSec - Should prevent users from sending more fees than required in `Factory.sol` #82

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

GimelSec

medium

Should prevent users from sending more fees than required in Factory.sol

Summary

If a user accidently sends more fees than required during deployment, Factory contract just accepts it, resulting in loss of funds for the user.

Vulnerability Detail

Factory.sol modifier paidOnly(uint256 fee) will enforce a minimum payment for the function call. This modifier is used by call and deploy functions. In the call function, users may need to send more msg.value than callFee and pass remaining values to _call. But in the deploy function, it's not necessary to pass the remaining msg.value to _deploy. If a user sends msg.value more than deploymentFee, the user will lose these remaining ETH.

Impact

Users will lose funds if more than required ETH are sent to deploymentFee accidentally.

Code Snippet

https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/Factory.sol#L136

Tool used

Manual Review

Recommendation

When calling the deploy function, it should check that msg.value == deploymentFee:

    function deploy(string calldata name, bytes calldata initdata)
        external
        payable
        exactPaidOnly(deploymentFee)

    modifier exactPaidOnly(uint256 fee) {
        require(msg.value == fee, "wrong payment");
        _;
    }