It concludes the ratio between assets and shares is constant. But if assets or shares change between checkpoints, the conversion ratio would be stale

Summary

It assumes the ratio between assets and shares is constant. But if assets or shares change between checkpoints, the conversion ratio would be stale.

Vulnerability Detail

The convertToShares() and convertToAssets() functions rely on the total assets and total shares from the latest checkpoint: Link 1 and Link 2

The totalAssets() and totalShares() functions get the latest checkpoint values: Link 3 and Link 4

However, between checkpoints, assets and shares can change via deposits, redemptions, trading gains/losses, etc. So the ratio becomes stale.

The totalAssets() and totalShares() functions return the totals from the latest checkpoint. But if assets or shares have changed since that checkpoint, the conversion ratio would be stale. For example: • Checkpoint 1: 100 assets, 100 shares • Conversion ratio: 1 asset = 1 share • Checkpoint 2: 200 assets, 100 shares • But totalAssets and totalShares still return 100 assets and 100 shares from Checkpoint 1 • So convertToShares(50) would incorrectly return 50 shares instead of 25 shares

Impact

This could allow attackers to get more shares than they should when depositing assets, or redeem more assets than they should when withdrawing shares.

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 https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L111-L116 https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L120-L125

Tool used

Manual Review

Recommendation

Only allow conversions at checkpoints.
Maintain a running balance of assets and shares that gets updated on each operation.
Use the running balances for conversions, and reset them at each checkpoint. This would ensure the conversion ratio stays up-to-date between checkpoints.

sherlock-audit / 2023-09-perennial-judging

feelereth - It concludes the ratio between assets and shares is constant. But if assets or shares change between checkpoints, the conversion ratio would be stale #49