sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

feelereth - It assumes totalAssets and totalShares cannot be 0. This could happen if the contract is just initialized. #50

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

feelereth

medium

It assumes totalAssets and totalShares cannot be 0. This could happen if the contract is just initialized.

Summary

In the convertToShares and convertToAssets functions, it is possible for _totalAssets or _totalShares to be 0 if the contract was just initialized.

Vulnerability Detail

the convertToShares() and convertToAssets() functions could divide by 0 if _totalAssets or _totalShares is 0. Here is the relevant code: Link1 and link2 If _totalAssets or _totalShares is 0, it would cause a division by 0 error and revert the transaction.

This could happen right after deploying the contract, before any assets or shares have been minted. Or if assets and shares have been burned down to 0 for some reason.

An attacker could intentionally manipulate assets and shares to 0 to make the contract unusable.

Impact

An attacker could intentionally manipulate assets and shares to 0 to make the contract unusable.

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L130-L134 https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L139-L143

Tool used

Manual Review

Recommendation

The contract should check for 0 values and handle them safely

sherlock-admin commented 11 months ago

4 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid because there is no impact and no scenario when this can happen

n33k commented:

invalid, not convincing without PoC

darkart commented:

Informational

polarzero commented:

Invalid. While this is true, this would happen soon enough to not incur a significant loss of funds.