code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

`SpeedBumpPriceGate` does not refund excess ETH payment #271

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/e22a562c01c533b8765229387894cc0cb9bed116/contracts/SpeedBumpPriceGate.sol#L65-L82

Vulnerability details

The FixedPricePassThruGate accepts ETH amounts greater than or equal to the calculated price, and forwards the full amount to the gate's configured beneficiary address. However, there is no mechanism to refund these excess payments, and no guarantee that the beneficiary will refund them. Moreover, there is no record stored or event emitted recording the price paid by user address.

SpeedBumpPriceGate#passThruGate

    function passThruGate(uint index, address) override external payable {
        uint price = getCost(index);
        require(msg.value >= price, 'Please send more ETH');
        // bump up the price
        Gate storage gate = gates[index];
        // multiply by the price increase factor
        gate.lastPrice = (price * gate.priceIncreaseFactor) / gate.priceIncreaseDenominator;
        // move up the reference
        gate.lastPurchaseBlock = block.number;

        // pass thru the 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: msg.value}("");
            require(sent, 'ETH transfer failed');
        }
    }

Suggestion: Forward only ETH equal to the mint price to the beneficiary address. Store the amount paid by user address and allow users to reclaim excess ETH.

illuzen commented 2 years ago

Duplicate #48

gititGoro commented 2 years ago

Increasing severity as user funds are lost.