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

Incorrect `decimalOffset` leading to miscalculated vault decimals and share ratios

alexczm

Summary

Incorrect decimalOffset causes the share-per-asset ratio to be 1 instead of 10^offset.
The vault's decimals function returns 18, while in reality has same decimals as the underlying asset.

Vulnerability Details

When a ZlpVault is initialized, it sets decimalOffset. Since the SYSTEM_DECIMALS is 18, it's safe to assume decimalOffset will be set to 18 - underlying token decimals.

ZlpVault overrides _decimalsOffset(), which is used by the OpenZeppelin (OZ) implementation to calculate the vault's decimals, resulting in 18.

function decimals() public view virtual override(IERC20Metadata, ERC20Upgradeable) returns (uint8) {
ERC4626Storage storage $ = _getERC4626Storage();
return $._underlyingDecimals + _decimalsOffset();
}

The ERC-4626 OZ implementation uses decimalOffset to protect against so-called "inflation attacks."

The protocol overrides _convertToAssets and _convertToShares to implement custom logic.

These functions call getIndexTokenSwapRate and getVaultAssetSwapRate, respectively(1, 2). However, the problem is that these functions use a different value for decimalOffset.

function getIndexTokenSwapRate() ... {
// Fetch storage slot for vault by ID
Vault.Data storage vault = Vault.loadExisting(vaultId);
...
// @audit: Incorrect decimal offset value: `18 - 18 = 0`
// 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 cause issues
uint256 previewAssetsOut = sharesIn.mulDiv(
totalAssetsMinusVaultDebt,
@> IERC4626(vault.indexToken).totalSupply() + 10 ** decimalOffset,
MathOpenZeppelin.Rounding.Floor
);
}

The newly calculated decimalOffset is 0, meaning the share-per-asset ratio becomes 1 instead of 10^offset.

While this still offers protection against inflation attacks (if the offset is 0, the attacker's loss is at least equal to the users', as noted here), the vault's decimals function returns 18, whereas in reality, it matches the underlying asset's decimals.

Impact

  • Smaller protection against inflation attack for vaults with assets with less than 18 decimals;

  • Vault decimals misrepresentation. It could represent an external risk in case of vault's integration with other protocols.

Tools Used

Recommendations

  • Ensure consistency in how decimalOffset is used acros functions.

  • Modify getIndexTokenSwapRate and getVaultAssetSwapRate to correctly derive decimalOffset from the underlying token, or better, read it directly from zlpVault.

  • Consider setting zlpVaultStorage.decimalsOffset to SYSTEM_DECIMALS - underlyingAssetDecimals.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[INVALID] `decimalOffset` lowers the inflation attack protection of OZ's ERC4626Upgradable implementation

Appeal created

alexczm Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect decimal offset calculation in VaultRouterBranch reduces donation attack protection to 1 for all tokens

Support

FAQs

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

Give us feedback!