LiquidityPool's Non-Compliance with ERC-4626 Due to Rounding Issues

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/LiquidityPool.sol#L160-L162
https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L392
https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L616
https://github.com/code-423n4/2023-09-centrifuge/blob/main/src/InvestmentManager.sol#L692

Vulnerability details

Impact

Other protocols that integrate with centrifufge may wrongly assume that the functions are EIP-4626 compliant.

Proof of Concept

EIP 4626's Security Considerations (https://eips.ethereum.org/EIPS/eip-4626).

Finally, ERC-4626 Vault implementers should be aware of the need for specific, opposing rounding directions across the different mutable and view methods, as it is considered most secure to favor the Vault itself during calculations over its users:

If (1) it’s calculating how many shares to issue to a user for a certain amount of the underlying tokens they provide or (2) it’s determining the amount of the underlying tokens to transfer to them for returning a certain amount of shares, it should round down.
If (1) it’s calculating the amount of shares a user has to supply to receive a given amount of the underlying tokens or (2) it’s calculating the amount of underlying tokens a user has to provide to receive a certain amount of shares, it should round up.

The LiquidityPool, which claims to adhere to the ERC-4626 standard, has been found to have a significant discrepancy in its internal logic. The primary functions, previewMint and previewWithdraw, which are expected to round up as per the standard, are currently rounding down.

Tools Used

VS Code

Recommended Mitigation Steps

previewMint and previewWithdraw should round up

Assessed type

ERC4626

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #34

[c4-judge]

gzeon-c4 marked the issue as satisfactory