Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

LPs will not recieve their full amount of collateral on redeem

dav1dstefanow

Summary

Protocol assumes that decimals of the share token during VaultRouterBranch::getVaultAssetSwapRate and VaultRouterBranch::getIndexTokenSwapRate is always 18. That would lead to precision loss and liquidity providers who want to redeem their collateral will take much less than their initial deposit while the rest gets stuck.

Vulnerability Details

Let's look at VaultRouterBranch::getIndexTokenSwapRate :

function getIndexTokenSwapRate(
uint128 vaultId,
uint256 sharesIn,
bool shouldDiscountRedeemFee
)
public
view
returns (UD60x18 assetsOut)
{
...
// get decimal offset
uint8 decimalOffset = Constants.SYSTEM_DECIMALS - IERC20Metadata(vault.indexToken).decimals(); <===
​
// Get the asset amount out for the input amount of shares, taking into account the vault's debt
// See {IERC4626-previewRedeem}
// `IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset` could lead to problems
uint256 previewAssetsOut = sharesIn.mulDiv(
totalAssetsMinusVaultDebt,
IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset, <====
MathOpenZeppelin.Rounding.Floor
);
​
...
}
​

We get the decimalOffset of our indexToken by subtracting SYSTEM_DECIMALS, which is 18, by indexToken decimals that can be any given number. It can be any given number because this functions:

File::ERC4626Upgradeable
function decimals() public view virtual override(IERC20Metadata, ERC20Upgradeable) returns (uint8) {
ERC4626Storage storage $ = _getERC4626Storage();
return $._underlyingDecimals /* which are 6 */ + _decimalsOffset() // which is 0;
}
File::ERC4626Upgradeable
function _decimalsOffset() internal view virtual returns (uint8) {
return 0;
}

Were never overwritten and so if the underlying token has 6 decimals, decimals() of the index token will equal 6 + 0 = 6 decimals
decimalOffset will be 12. By adding it to indexToken total supply will equal much different number, but it's still less than what should be the correct result.

Example:
asset token decimals = 6;
indexToken decimals = asset decimals + decimalOfset = 6 + 0 =6;
sharesIn = 20e18;
Let's consider totalSupply = 1320e6;
totalAssetsMinusVaultDebt = 1800e18;
previewAssetOut = (20e18 * 1800e18) / (1320e6 + 1e12) = 3.5952542643710302401e28; a number way bigger than it should
Instead, if we do the math: (20e18 1800e18) / (1320e6 1e12) = 2.7272727272727272727e19 or 27 dollars per share - what should be the cost per share
verification: (20 * * 1800) / 1320 = 27.272727272727272727

Impact

LPs losing huge part of their funds forever on redeem

Tools Used

Manual

Recommendations

Multiply indexToken total supply by decimalOffset so it would always equal 18 decimals

function getVaultAssetSwapRate(
uint128 vaultId,
uint256 assetsIn,
bool shouldDiscountDepositFee
)
public
view
returns (UD60x18 sharesOut)
{
...
​
// get decimal offset
uint8 decimalOffset = Constants.SYSTEM_DECIMALS - IERC20Metadata(vault.indexToken).decimals();
​
// Get the shares amount out for the input amount of tokens, taking into account the unsettled debt
// See {IERC4626-previewDeposit}.
// `IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset` could lead to problems
uint256 previewSharesOut = assetsIn.mulDiv(
- IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset,
+ IERC4626(vault.indexToken).totalSupply() * 10 ** decimalOffset,
totalAssetsMinusVaultDebt,
MathOpenZeppelin.Rounding.Floor
);
​
...
}
​
function getIndexTokenSwapRate(
uint128 vaultId,
uint256 sharesIn,
bool shouldDiscountRedeemFee
)
public
view
returns (UD60x18 assetsOut)
{
...
​
// get decimal offset
uint8 decimalOffset = Constants.SYSTEM_DECIMALS - IERC20Metadata(vault.indexToken).decimals();
​
// Get the asset amount out for the input amount of shares, taking into account the vault's debt
// See {IERC4626-previewRedeem}
// `IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset` could lead to problems
uint256 previewAssetsOut = sharesIn.mulDiv(
totalAssetsMinusVaultDebt,
- IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset,
+ IERC4626(vault.indexToken).totalSupply() * 10 ** decimalOffset,
MathOpenZeppelin.Rounding.Floor
);
​
...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

getVaultAssetSwapRate and getIndexTokenSwapRate inside VaultRouterBranch add the decimalOffset instead of multiplying it

Appeal created

oxelmiguel Auditor
4 months ago
oxelmiguel Auditor
4 months ago
inallhonesty Lead Judge
4 months ago
inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

getVaultAssetSwapRate and getIndexTokenSwapRate inside VaultRouterBranch add the decimalOffset instead of multiplying it

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.