Fractional v2 contest - exd0tpy's results

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/Buyout.sol#L86-L87

Vulnerability details

Impact

Buyout proposer's ether is lock inside contract or unfair distribution in some edge case.

Proof of Concept

Ether lock case

1. Assume Alice want to buyout NFT that total supply is 100,000 fractional token.
2. Alice has 999 of fractional token, start buyout pool with 100ETH of buyout price.

        uint256 buyoutPrice = (msg.value * 100) /
            (100 - ((depositAmount * 100) / totalSupply));
        uint256 fractionPrice = buyoutPrice / totalSupply;

3. Pool's buyoutPrice will (100ETH * 100) / (100 - ((999*100)/100000)) = 100 and fractionPrice will 100/100000 = 0.001ETH
4. Bob sell 99001 fractional token for 99001*0.001ETH. Pool has 0.999ETH left.
5. Pool end and Alice withdraw NFT.
6. Nobody can execute cash(), because there is no more fractional token for that NFT.

Unfair distribution

1. Assume same pool state. fractionPrice is 0.001ETH.
2. Bob sell 99000 fractional token for 99000*0.001ETH. Pool has 1ETH left.
3. Eve has 1 fractional token but didn't sell it.
4. Pool end and Alice withdraw NFT.
5. When Alice end pool, all of Alice's token are burnt so totalSupply of token is only 1.
6. Eve cash out for 1 token.

uint256 buyoutShare = (tokenBalance * ethBalance) /
            (totalSupply + tokenBalance);

7. buyoutShare will `(1 * 1ETH) / (1 + 0) = 1ETH. Eve get 1ETH for just 1 token.

Tools Used

VS Code

Recommended Mitigation Steps

Use larger precision for calculate buyoutPrice.

Proper upper limit on ERC1155 royalty

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/FERC1155.sol#L223

setRoyalties in FERC1155.sol has no upper limit. It could be set more than 100%.

Use reentrancy guard and set variable before transfer.

In buyFractions() in Buyout.sol has user callback.

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L168-L174

update ethBalance before transfer.

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L176

And use reentrancy guard for security.