`FixedPricePassThruGate` locks excess ETH payments

[code423n4]

Lines of code

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

Vulnerability details

The FixedPricePassThruGate accepts ETH amounts greater than or equal to the calculated price, but only forwards an amount exactly equal to the calculated price to the configured beneficiary address. Excess ETH sent through the gate will be permanently locked in the contract.

Scenario:

Alice calls MerkleIdentity#withdraw to mint a token configured with a FixedPricePassThruGate and a price of 1 ETH. She sends 2 ETH with her transaction.

The withdraw function forwards all 2 ETH to passThruGate:

MerkleIdentity.sol#L132-L133

        // check that the price is right
        IPriceGate(tree.priceGateAddress).passThruGate{value: msg.value}(tree.priceIndex, msg.sender);

The FixedPricePassThruGate checks that the msg.value amount is greater than or equal to gate.ethCost on line 48. Since the forwarded 2 ETH is greater than the 1 ETH gate.ethCost, this check succeeds. The gate then sends gate.ethCost to the beneficiary on line 53.

FixedPricePassThruGate#passThruGate

    function passThruGate(uint index, address) 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');
        }
    }

Impact: The remaining msg.value - gate.ethCost amount (1 ETH in the above example) is locked in the contract, with no mechanism to retrieve it.

Suggestion: require an exact payment amounts in the FixedPricePassThruGate rather than accepting excess ETH.

[illuzen]

Duplicate #49

[gititGoro]

Duplicate of #48

[gititGoro]

Upgraded severity: lost funds.