code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/Crowdfund.sol#L339-L376

Vulnerability details

Impact

Function _getFinalContribution() loops throw all contributions which have been added by contributor. If there are too many contributions then _getFinalContribution() will be reverted due to reaching the block gas limit.

Function _burn() calls _getFinalContribution(). If _getFinalContribution reverts due to reaching the block gas limit, the _burn() will not be available to be called because contributor can't reduce the number of contributions.

If _burn() will be reverted then user's ETH will be locked.

Proof of Concept

If there are too many contributions then _getFinalContribution() will be reverted due to reaching the block gas limit.

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/crowdfund/Crowdfund.sol#L339-L376

function _getFinalContribution(address contributor)
        internal
        view
        returns (uint256 ethUsed, uint256 ethOwed, uint256 votingPower)
    {
        uint256 totalEthUsed = _getFinalPrice();
        {
            Contribution[] storage contributions = _contributionsByContributor[contributor];
            uint256 numContributions = contributions.length;
            for (uint256 i = 0; i < numContributions; ++i) {
                Contribution memory c = contributions[i];
                if (c.previousTotalContributions >= totalEthUsed) {
                    // This entire contribution was not used.
                    ethOwed += c.amount;
                } else if (c.previousTotalContributions + c.amount <= totalEthUsed) {
                    // This entire contribution was used.
                    ethUsed += c.amount;
                } else {
                    // This contribution was partially used.
                    uint256 partialEthUsed = totalEthUsed - c.previousTotalContributions;
                    ethUsed += partialEthUsed;
                    ethOwed = c.amount - partialEthUsed;
                }
            }
        }
        // one SLOAD with optimizer on
        address splitRecipient_ = splitRecipient;
        uint256 splitBps_ = splitBps;
        if (splitRecipient_ == address(0)) {
            splitBps_ = 0;
        }
        votingPower = ((1e4 - splitBps_) * ethUsed) / 1e4;
        if (splitRecipient_ == contributor) {
            // Split recipient is also the contributor so just add the split
            // voting power.
            votingPower += (splitBps_ * totalEthUsed + (1e4 - 1)) / 1e4; // round up
        }
    }

Function _contribute() has no limits when adding a new contribution.

Function _burn() calls _getFinalContribution() here. If _getFinalContribution() reverts due to reaching the block gas limit, the _burn() will be revered too.

Tools Used

Manual review

Recommended Mitigation Steps

Limit number of contributions that can be added.

merklejerk commented 1 year ago

Duplicate of #207

HardlyDifficult commented 1 year ago

See dupe for context.

Merging with the warden's QA report #318

code-423n4 / 2022-09-party-findings