code-423n4 / 2023-10-party-findings

6 stars 4 forks source link

InitialETHCrowdfund.sol#initialize - If the crowdfund has ETH inside it before it gets initialized, it won''t go to the initial contribution #197

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/crowdfund/InitialETHCrowdfund.sol#L140-L151

Vulnerability details

Impact

During initialization of a InitialETHCrowdfund, we have someone called initialContributor. This is an address that will be accounted for an initial contribution to the crowdfund, if set. The point of this initial contributor is to kickstart the crowdfund with some ETH.

 uint96 initialContribution = msg.value.safeCastUint256ToUint96();
        if (initialContribution > 0) {
            // If this contract has ETH, either passed in during deployment or
            // pre-existing, credit it to the `initialContributor`.
            _contribute(
                crowdfundOpts.initialContributor,
                crowdfundOpts.initialDelegate,
                initialContribution,
                0, 
                ""
            );
        }

As you can see the comment states: "If this contract has ETH, either passed in during deployment or pre-existing, credit it to the initialContributor."

However, the initial contributor is only credited with a contribution if he passed in msg.value. Nowhere do we check if the contract already has a balance. If it has a balance it should be credited to the initial contributor as well.

Proof of Concept

Example:

Tools Used

Manual Review

Recommended Mitigation Steps

Add the following inside initialize.

uint96 initialContribution = msg.value.safeCastUint256ToUint96() + address(this).balance;
        if (initialContribution > 0) {
            // If this contract has ETH, either passed in during deployment or
            // pre-existing, credit it to the `initialContributor`.
            _contribute(
                crowdfundOpts.initialContributor,
                crowdfundOpts.initialDelegate,
                initialContribution,
                0, 
                ""
            );
        }

Assessed type

Invalid Validation

ydspa commented 1 year ago

QA: NC

c4-pre-sort commented 1 year ago

ydspa marked the issue as insufficient quality report

c4-judge commented 1 year ago

gzeon-c4 changed the severity to QA (Quality Assurance)