code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

ERC4626 Vault may be susceptible to underlying asset and vault token different decimals #64

Closed c4-bot-3 closed 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L472

Vulnerability details

Impact

IERC4626 vaults may be from good protocols or compliant to IERC4626 have the best intentions but still overlook an issue in pricing when doing calculations if decimals for underlying asset differ from vault token decimals

This is not uncommon as popular defi stablecoins USDC, USDT have 6 decimals and the vault token may be set as common 18 decimals and the conversion calculations omit by error or unaware the impact of not taking into account decimal differences leading to losses to the users or the protocol

The ERC4626 vault must always ensure underlying assets decimals match vault token decimals or account for in price, conversion calculations via 10 ** IERC20Metadata(underlyingToken).decimals()

Proof of Concept

EIP-4626 does not require the decimals must be the same as the underlying tokens' decimals

The conversions to redeem of ERC4626 will be significantly underestimated when the underlying token's decimals > ERC4626's decimals, and be significantly overestimated when the underlying token's decimals < ERC4626's decimals.

Vault that is used that does not take into account decimals correctly in conversions, redemptions, pricing etc may result in lower than expected or higher than expected value - leading to problems in this protocol for example lower than expected values disdvantage users and higher than expected values disadvantage protocol

Tools Used

Manual Analysis

Recommended Mitigation Steps

It is recommended to make use of ibts vaults that have proper implementations that safeguard against decimals differences It is recommended this risk be communicated to users so they are aware before depositing underlying or ibts of specific ERC4626 vault so they can verify there will not be this issue

Assessed type

ERC4626

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as primary issue

gzeon-c4 commented 8 months ago

lack poc

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid