search

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

0 stars 0 forks source link

Not update `ethBalance` after users cash out in `Migration` may cause loss of funds #626

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

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;
ecmendenhall commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-07-fractional-findings/issues/440