Any user can create distribution to gain funds from TokenDistributor contract

[code423n4]

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/distribution/TokenDistributor.sol#L98-L115
https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/distribution/TokenDistributor.sol#L118-L135

Vulnerability details

Impact

TokenDistributor.createNativeDistribution() and TokenDistributor.createErc20Distribution() allows anyone to create token distribution for eth or erc20 token. A user can monitor the erc20 token or eth balance of the token distributor balance and create a distribution for themselves whenever the tokens balance is greater than _storedBalances

This can be quite profitable for users monitoring the contract and they gain funds for free at the expense of any user who sent funds direct to the contract.

Once the distribution has been created, the user can call claim() or claimFee() depending on the user input during the distribution creation and be transferred the funds balance difference.

Proof of Concept

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/distribution/TokenDistributor.sol#L190-L222

1. TokenDistributor contract has balance of 20 eth.
2. Bob monitors the contract and notices that some user had unknowingly or intentionally sent 1 eth to the contract.
3. Bob calls createNativeDistribution() ensuring that they are the recipient and feeBps is 1e4
4. The distribution is created
5. Bob calls TokenDistributor.claimFee() with his address as recipient and a custom DistributionInfo such that;
   info.tokenType = TokenType.Native
   info.token = NATIVE_TOKEN_ADDRESS
   info.feeRecipient = Bob
   info.fee = 1 eth
6. Bob would receive 1eth as the input would pass and the _transfer() called made to send eth to Bob as the receiver.

Tools Used

Manual review

Recommended Mitigation Steps

Apply necessary access controls

[merklejerk]

This is unsanctioned usage. The distributor is not supposed to be used in this way. You must transfer and create a distribution in the same transaction, or else you risk losing access to your transfer.

[trust1995]

@merklejerk Do you consider this a high severity finding? Because using sponsor-ack is confirming the severity, which does not sound like the case here (requires direct misuse of the API)

[HardlyDifficult]

Agree with the sponsor here - that requirement is noted in comments. The report aims to protect against user error which is Low risk.

Downgrading and converting this into a QA report for the warden.

[HardlyDifficult]

Merging with #310