FixedPricePassThruGate: Excess ether lock in the contract

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/e22a562c01c533b8765229387894cc0cb9bed116/contracts/FixedPricePassThruGate.sol#L46-L56

Vulnerability details

Impact

The passThruGate function of the FixedPricePassThruGate contract is used to charge NFT purchase fees.
When msg.value> gate.ethcost, the excess ether did not return to the user, but was locked in the contract.

Proof of Concept

https://github.com/code-423n4/2022-05-factorydao/blob/e22a562c01c533b8765229387894cc0cb9bed116/contracts/FixedPricePassThruGate.sol#L46-L56

Tools Used

None

Recommended Mitigation Steps

-   function passThruGate(uint index, address) override external payable {
+  function passThruGate(uint index, address payer) override external payable {
        Gate memory gate = gates[index];
        require(msg.value >= gate.ethCost, 'Please send more ETH');

        // pass thru ether
        if (msg.value > 0) {
            // use .call so we can send to contracts, for example gnosis safe, re-entrance is not a threat here
            (bool sent, bytes memory data) = gate.beneficiary.call{value: gate.ethCost}("");
            require(sent, 'ETH transfer failed');
        }
+      if (msg.value - gate.ethCost > 0){
+         (bool sent, bytes memory data) = payer.call{value: msg.value - gate.ethCost}("");
+          require(sent, 'ETH transfer failed');}
    }

[illuzen]

Valid

[illuzen]

code-423n4/2022-05-factorydao#4

[gititGoro]

Duplicate of #48

[gititGoro]

The duplicate link is changed as the issue is the same but for a different sub contract. Wardens who group similar contracts would be penalized if these were recognized as distinct.