Fractional v2 contest - minhquanym's results

Lines of code

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

Vulnerability details

Impact

In Buyout.start() function, fractionPrice is calculated from msg.value and depositAmount, both can be changed by sender. And there is no check if fractionPrice is bigger than 0.

So in case fractionPrice = 0, users may sell and buy fractions with zero ETH. For example, in case selling fraction, users will receive nothing when sell their fractions.

Similarly in case buying fractions, users can buy all fractions without any ETH because of this check

Proof of Concept

1. In case buyoutPrice < totalSupply, fractionPrice will be round down to 0.

2. In sellFractions() function, ethAmount is calculated as

ethAmount = fractionPrice * _amount = 0 (fractionPrice = 0)

So sellers call sellFraction() will receive nothing.

3. In buyFractions() function, buyers can send 0 ETH with any _amount input and this check is still passed, allow buyers to get all fractions for free

if (msg.value != fractionPrice * _amount) revert InvalidPayment();

Tools Used

Manual Review

Recommended Mitigation Steps

Check if fractionPrice > 0 in start() function

require(fractionPrice != 0);

Lines of code

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

Vulnerability details

Impact

In Buyout.cash() function, users will burn their fractions to cash out ETH. The ETH amount is proportionate the number of fractions they have. So when someone burn fractions, total amount of ETH (ethBalance) should be updated accordingly. But in cash() function, there is no update for ethBalance.

The result is some last users may unable to cash out because actual ETH balance of vault is zero.

Proof of Concept

1. Vault currently has ethBalance = 1e18, totalSupply = 1e18.
2. Alice cash out 5e17 fractions token. She will receive 5e17 wei. Now, totalSupply = 5e17 but ethBalance is not updated and still = 1e18. But actually the vault ETH balance is only 5e17 now
3. Alice continue cash out 4e17 fractions token. She will receive

(tokenBalance * ethBalance) / totalSupply = (4e17 * 1e18) / 5e17 = 8e17 > 5e17

So Alice TX will be reverted because there is not enough balance.

Tools Used

Manual Review

Recommended Mitigation Steps

Update ethBalance after cash out

buyoutInfo[_vault].ethBalance -= buyoutShare;