search

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

2 stars 0 forks source link

Any user can create distribution to gain funds from TokenDistributor contract #308

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

Merging with https://github.com/code-423n4/2022-09-party-findings/issues/310