search

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

0 stars 0 forks source link

Proposal commitment takes all fractional balance in Migration contract #619

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#L206-L213 https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L72-L82

Vulnerability details

Impact

The Migration contract holds all fractional token balance of all proposals.

Let's suppose a single vault has multiple proposals going on, and one gets committed. If the target price is satisfied, then a buyout starts

// Checks if the current price is greater than target price of the proposal
if (currentPrice > proposal.targetPrice) {
    // Sets token approval to the buyout contract
    IFERC1155(token).setApprovalFor(address(buyout), id, true);
    // Starts the buyout process
    IBuyout(buyout).start{value: proposal.totalEth}(_vault);
    proposal.isCommited = true;
    started = true;
}

Inside Buyout.start, all balance of the contract is taken:

// Gets total balance of fractional tokens owned by caller
uint256 depositAmount = IERC1155(token).balanceOf(msg.sender, id);

// Transfers fractional tokens into the buyout pool
IERC1155(token).safeTransferFrom(
    msg.sender,
    address(this),
    id,
    depositAmount,
    ""
);

Basically the buyout starts using also the fractional balance allocated for other proposals.

Proof of Concept

Alice starts a malicious proposal (id = 1). After a while a good proposal (id = 2) picks up, and users joins it by depositing their tokens.

Alice can call commit for proposal 1, starting the buyout (suppose her target price was very low). Since the buyout starts with also the balance for proposal 2, it's possible that her malicious proposal passes the buyout and is settled.

Recommended Mitigation Steps

Buyout.start(...) should have an input argument for the token balance to transfer. Migration.commit(...) should pass migrationInfo[_vault][_proposalId].totalFractions.

HardlyDifficult commented 2 years ago

Duping to https://github.com/code-423n4/2022-07-fractional-findings/issues/183