search

code-423n4 / 2022-07-fractional-findings

0 stars 0 forks source link

User can increase migration's proposal total tokens artificially #632

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L292-L326

Vulnerability details

Impact

There are two functions that users can use to withdraw deposited ether/tokens from a joined proposal: leave and withdrawContribution.

leave correctly decreases the total contribution from a proposal alongside the user's one:

// Updates fraction balances of the proposal and caller
uint256 amount = userProposalFractions[_proposalId][msg.sender];
proposal.totalFractions -= amount;
userProposalFractions[_proposalId][msg.sender] = 0;
// Updates ether balances of the proposal and caller
uint256 ethAmount = userProposalEth[_proposalId][msg.sender];
proposal.totalEth -= ethAmount;
userProposalEth[_proposalId][msg.sender] = 0;

However, withdrawContribution only zeroes the user's values, and not the totals. It's possible then to artificially increase a proposal balance, without truly depositing that quantity of ETH/tokens. This may lead to malicious proposal passing and/or stealing of assets from the migration contracts.

Proof of Concept

Alice starts a proposal. 1) She joins it with 1 ETH;

With this method it's possible to increase the proposal's balance without spending anything. Alice can stop until her proposal reaches the true address(this).balance, so she can call commit stealing ETH from other proposals.

Same argument works for fractional tokens instead of ETH.

Recommended Mitigation Steps

Remove withdrawContribution, since it serves the same purpose as leave.

mehtaculous commented 2 years ago

Duplicate of #27