Fractional v2 contest - 0xsolstars's results

Buyout can be successful and proposer loses initial buyout deposit and can't obtain tokens

Example Scenario

Consider the following case where Alice owns 2 erc1155s (the total supply).

1. She transfers 1 to Bob
2. Bob initializes buyout with his 1 erc1155 and msg.value = 1 wei.
3. Alice buys Bob's fractions for 1 wei.
4. Alice the sells both her fractions into the vault.
5. Buyout ends and would be successful.

Bob has no way of obtaining tokens since he does not own any. The test case below demonstrates the scenario.

function testEndSuccessfull2() public {
      // total_supply = 10000
      // half_supply = 5000
      // set up vault with alice owning 2
      // transfer from alice to bob 1 erc1155
      initializeBuyout(alice, bob, 2, 1, true);
      // bob trying to buy out Alice
      bob.buyoutModule.start{value: 1 wei}(vault);

      alice.buyoutModule.buyFractions{value: 1 wei}(vault, 1);
      alice.buyoutModule.sellFractions(vault, 2);
      assertEq(getETHBalance(alice.addr), 100 ether + 1 wei);
      vm.warp(rejectionPeriod + 1);

      assertEq(getFractionBalance(bob.addr), 0);
      assertEq(getFractionBalance(buyout), 2);

      bob.buyoutModule.end(vault, burnProof);

      assertEq(getETHBalance(buyout), 0 ether);
      assertEq(getETHBalance(bob.addr), 100 ether);
      assertEq(getFractionBalance(bob.addr), 0);
      assertEq(getFractionBalance(buyout), 0);
    }

Recommendation:

Prevent a user from selling owned fractions into the buyout pool if user owns > 50% of the total supply.

Spelling Errors

Spelling Errors: Succesful -> Successful

I believe this is in other files as well.

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/test/Buyout.t.sol#L320 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/test/Buyout.t.sol#L336

No Address Validation

When transferring ownership to an account, there is no validation done on the account that it is being transferred to. Could validate a 0 address check here to be safer.

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

Add check for 0 amount in buy and sell fractions to avoid executing unnecessary logic. For example:

if (_amount) == 0) revert ZeroAmount();

could add the above in the right before the line in the below snippets.

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

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