In some cases, crowd fund can not be finalized due to minContribution restriction.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/ETHCrowdfundBase.sol#L196-L273

Vulnerability details

Impact

Crowd fund can not be finalized in some cases.

Since this affects the core feature of the party protocol, we are raising this as high.

Proof of Concept

An user can contribute to a crowdfund by calling the functions contribute, batchContribute, batchContributeFor functions in InitialETHCrowdfund.sol contract.

The function _contribute checks for allowed user by the keeper. Check

voting power will be calculated by calling the function _processContribution.

when we look at the function _processContribution

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/ETHCrowdfundBase.sol#L196-L273

    function _processContribution(
        address payable contributor,
        address delegate,
        uint96 amount
    ) internal returns (uint96 votingPower) {
        address oldDelegate = delegationsByContributor[contributor];
        if (msg.sender == contributor || oldDelegate == address(0)) {
            // Update delegate.
            delegationsByContributor[contributor] = delegate;
        } else {
            // Prevent changing another's delegate if already delegated.
            delegate = oldDelegate;
        }


        emit Contributed(msg.sender, contributor, amount, delegate);


        // OK to contribute with zero just to update delegate.
        if (amount == 0) return 0;


        // Only allow contributions while the crowdfund is active.
        CrowdfundLifecycle lc = getCrowdfundLifecycle();
        if (lc != CrowdfundLifecycle.Active) {
            revert WrongLifecycleError(lc);
        }


        // Check that the contribution amount is at or below the maximum.
        uint96 maxContribution_ = maxContribution;
        if (amount > maxContribution_) {
            revert AboveMaximumContributionsError(amount, maxContribution_);
        }


        uint96 newTotalContributions = totalContributions + amount;
        uint96 maxTotalContributions_ = maxTotalContributions;
        if (newTotalContributions >= maxTotalContributions_) {
            totalContributions = maxTotalContributions_;


            // Finalize the crowdfund.
            // This occurs before refunding excess contribution to act as a
            // reentrancy guard.
            _finalize(maxTotalContributions_);


            // Refund excess contribution.
            uint96 refundAmount = newTotalContributions - maxTotalContributions;
            if (refundAmount > 0) {
                amount -= refundAmount;
                payable(msg.sender).transferEth(refundAmount);
            }
        } else {
            totalContributions = newTotalContributions;
        }


        // Check that the contribution amount is at or above the minimum. This
        // is done after `amount` is potentially reduced if refunding excess
        // contribution. There is a case where this prevents a crowdfunds from
        // reaching `maxTotalContributions` if the `minContribution` is greater
        // than the difference between `maxTotalContributions` and the current
        // `totalContributions`. In this scenario users will have to wait until
        // the crowdfund expires or a host finalizes after
        // `minTotalContribution` has been reached by calling `finalize()`.
        uint96 minContribution_ = minContribution;
        if (amount < minContribution_) {
            revert BelowMinimumContributionsError(amount, minContribution_); --------------->> will revert here
        }


        // Subtract fee from contribution amount if applicable.
        address payable fundingSplitRecipient_ = fundingSplitRecipient;
        uint16 fundingSplitBps_ = fundingSplitBps;
        if (fundingSplitRecipient_ != address(0) && fundingSplitBps_ > 0) {
            // Removes funding split from contribution amount in a way that
            // avoids rounding errors for very small contributions <1e4 wei.
            amount = (amount * (1e4 - fundingSplitBps_)) / 1e4;
        }


        // Calculate voting power.
        votingPower = (amount * exchangeRateBps) / 1e4;


        if (votingPower == 0) revert ZeroVotingPowerError();
    }

The above function checks for min contribution and max contribution and ensure that the amount is not greater than max contribution and not less than min contribution.

when the sum of input amount and total amount (totalContributions + amount) exceeds the max contribution value, the amount which satisfy the max contribution is deducted from input amount and remaining is refunded to user.

if (newTotalContributions >= maxTotalContributions_) {
            totalContributions = maxTotalContributions_;

            // Finalize the crowdfund.
            // This occurs before refunding excess contribution to act as a
            // reentrancy guard.
            _finalize(maxTotalContributions_);

            // Refund excess contribution.
            uint96 refundAmount = newTotalContributions - maxTotalContributions;
            if (refundAmount > 0) {
                amount -= refundAmount;
                payable(msg.sender).transferEth(refundAmount);
            }
        } else {
            totalContributions = newTotalContributions;
        }

and then below checks will come

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/ETHCrowdfundBase.sol#L255-L258

        if (amount < minContribution_) {
            revert BelowMinimumContributionsError(amount, minContribution_);
        }

Lets see one case.

min amount = 3;
max amount = 4;
Total value = 10.

Following three transaction,

1st ==> 4 ETH

2nd ==> 4 ETH.

Now, remaining 2 would be utilized to finalize.

when another user send 4 ETH, the function first takes 2ETH and refund the remaining 2ETH. Finalized will be called.

Now, min amount check will come and revert due to min cap which is 3 ETH.

Tools Used

Manual review.

Recommended Mitigation Steps

We suggest to introduce a bool flag and set it true when finalize is called.

Once finalize is called, do not check for min contribution amount by using the above bool flag.

Assessed type

Invalid Validation

[c4-pre-sort]

ydspa marked the issue as duplicate of #552

[c4-pre-sort]

ydspa marked the issue as insufficient quality report

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Out of scope

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Out of scope

[c4-judge]

gzeon-c4 changed the severity to 2 (Med Risk)

[c4-judge]

gzeon-c4 marked the issue as not a duplicate

[c4-judge]

gzeon-c4 marked the issue as duplicate of #505

[aktech297]

@gzeon-c4
Thanks for your valuable time for judging this issue.

We believe that we have provided the adequate code reference and sample case that would simulate the actual issue.

From the submisstion,

Lets see one case.

min amount = 3;
max amount = 4;
Total value = 10.

Following three transaction,

1st ==> 4 ETH

2nd ==> 4 ETH.

Now, remaining 2 would be utilized to finalize.

when another user send 4 ETH, the function first takes 2ETH and refund the remaining 2ETH. Finalized will be called.

Now, min amount check will come and revert due to min cap which is 3 ETH.

We have not find any issue which are related to this issue in the known section.

we kindly request judge to give second look and share your feedback.

Thanks!

[gzeon-c4]

This submission lacked ANY reference to minTotalContribution, which is crucial to differentiate the new issue with the known issue
Also see #453 (comment)