Lines of code
<https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/ETHCrowdfundBase.sol#L317-L336&gt;
<https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/ETHCrowdfundBase.sol#L339-L359&gt;

Vulnerability details

Vulnerability details:

Details:

in ETHCrowdfundBase the crowdfund deployer can choose to implement fee’s that are given to a feeAdress with a certain percentage of the crowdfund ETH. the fee splitting is happening both in _finalize:

function _finalize(uint96 totalContributions_) internal {
        // Finalize the crowdfund.
        delete expiry;

        // Transfer funding split to recipient if applicable.
        address payable fundingSplitRecipient_ = fundingSplitRecipient;
        uint16 fundingSplitBps_ = fundingSplitBps;
        if (fundingSplitRecipient_ != address(0) && fundingSplitBps_ > 0) {
            totalContributions_ -= (totalContributions_ * fundingSplitBps_) / 1e4;
        }

        // Update the party's total voting power.
        uint96 newVotingPower = (totalContributions_ * exchangeRateBps) / 1e4;
        party.increaseTotalVotingPower(newVotingPower);

        // Transfer ETH to the party.
        payable(address(party)).transferEth(totalContributions_);

        emit Finalized();
    }

and in _processContribution:

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;
        }

which is a problem because the fees are basically taken twice. but also the function sendFundingSplit which is responsible for taking the fee split only takes half of the fees:

function sendFundingSplit() external returns (uint96 splitAmount) {
        // Check that the crowdfund is finalized.
        CrowdfundLifecycle lc = getCrowdfundLifecycle();
        if (lc != CrowdfundLifecycle.Finalized) revert WrongLifecycleError(lc);

        if (fundingSplitPaid) revert FundingSplitAlreadyPaidError();

        address payable fundingSplitRecipient_ = fundingSplitRecipient;
        uint16 fundingSplitBps_ = fundingSplitBps;
        if (fundingSplitRecipient_ == address(0) || fundingSplitBps_ == 0) {
            revert FundingSplitNotConfiguredError();
        }

        fundingSplitPaid = true;

        // Transfer funding split to recipient.
        splitAmount = (totalContributions * fundingSplitBps_) / 1e4;//@audit-issue doesn't take into account the fees taken from deposits -> funds stuck = total fees, can be saved by dao, but if disabled then it's stuck
        payable(fundingSplitRecipient_).transferEth(splitAmount);

        emit FundingSplitSent(fundingSplitRecipient_, splitAmount);
    }

basically leaving funds stuck in the crowdfund. and if emergencyExecute is disabled there is no way to get them out.

Impact:

the fees are taken twice which in itself is a vulnerability, accompanied by the sendFundingSplit only getting half of the fee out leaving the rest stuck, with no way of getting them out if emergencyExecute is disabled this deserves a high severity as it will happen in each crowdfund deployed and has clear loss of funds which is high impact.

Proof of Concept:

Bob deploys crowdfund with fee recipient set to himself and feeBps=500 with 10ETH to finalize crowdfund to simplify.

• Alice comes and contributes to the crowdfund with 10ETH finalizing it.
• she calls contribute in InitialETHCrowdfund which in turn calls _processContribution in ETHCrowdfundBase which takes the 5% fee = 0.5ETH
• because she finalized the crowdfund _finalize gets called which also takes 5% fee = 0.5ETH.
• now Bob comes and calls sendFundingSplit wich calculated the fee amount to take by splitAmount = (totalContributions * fundingSplitBps_) / 1e4;
• totalContributions = 10ETH , fundingSplitBps_ = 500
• so the split amount Bob is going to get is 0.5 ETH
• leaving another 0.5ETH stuck in the crowdfund with no way to get it out

Tools Used:

vscode

Recommended Mitigation Steps:

take the fee only once the crowdfund has finalized that way it will mitigate this vulnerability but also costs less gas.

Assessed type

Math

---

The text was updated successfully, but these errors were encountered:

All reactions