Rounding issue in convertToAssets

Summary

The mulDiv operation rounds down and creates problems in arithmetic.

Vulnerability Detail

convertToAssets is used in the Vault code to know how many assets correspond to a certain amount of shares. Essentially, the‌‌ number‌‌ of‌‌ assets that‌‌ a‌‌ user‌‌ will‌‌ receive‌‌ depends‌‌ on‌‌ the‌‌ total‌‌ shares,‌‌ the‌‌ total‌‌ amount‌‌ of‌‌ assets,‌‌ and‌‌ the‌‌ amount‌‌ of‌‌ shares.‌‌ The‌‌ total‌‌ asset amount‌‌ is‌‌ computed‌‌ by‌‌ the‌‌ following‌‌ code.

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L122-L126

However, the current implementation of convertToAssetsfunction will round down the number of shares returned due to how solidity handles Integer Division. Consequently, it will provide slightly fewer assets than expected for some amount of shares.

Impact

The function will return fewer number of assets than expected.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L122-L126

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/number/types/UFixed6.sol#L195-L197

Tool used

Manual Review + in-house tool

Recommendation

Ensure that the rounding of vault’s functions behave as expected. Alternatively, if such alignment of rounding could not be achieved due to technical limitation, at the minimum, document this limitation in the comment so that the developer performing the integration is aware of this.

sherlock-audit / 2023-07-perennial-judging

hassan-truscova - Rounding issue in convertToAssets #100