ERC4626 inflation attack mitigation

.changeset/perfect-insects-listen.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`ERC4626`: Add mitigation to the inflation attack through virtual shares and assets.

contracts/mocks/token/ERC4626OffsetMock.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../../token/ERC20/extensions/ERC4626.sol";
+
+abstract contract ERC4626OffsetMock is ERC4626 {
+    uint8 private immutable _offset;
+
+    constructor(uint8 offset_) {
+        _offset = offset_;
+    }
+
+    function _decimalsOffset() internal view virtual override returns (uint8) {
+        return _offset;
+    }
+}

contracts/token/ERC20/extensions/ERC4626.sol

@@ -17,28 +17,48 @@ import "../../../utils/math/Math.sol";
  * the ERC20 standard. Any additional extensions included along it would affect the "shares" token represented by this
  * contract and not the "assets" token which is an independent contract.
  *
- * CAUTION: When the vault is empty or nearly empty, deposits are at high risk of being stolen through frontrunning with
- * a "donation" to the vault that inflates the price of a share. This is variously known as a donation or inflation
+ * [CAUTION]
+ * ====
+ * In empty (or nearly empty) ERC-4626 vaults, deposits are at high risk of being stolen through frontrunning
+ * with a "donation" to the vault that inflates the price of a share. This is variously known as a donation or inflation
  * attack and is essentially a problem of slippage. Vault deployers can protect against this attack by making an initial
  * deposit of a non-trivial amount of the asset, such that price manipulation becomes infeasible. Withdrawals may
  * similarly be affected by slippage. Users can protect against this attack as well unexpected slippage in general by
  * verifying the amount received is as expected, using a wrapper that performs these checks such as
  * https://github.com/fei-protocol/ERC4626#erc4626router-and-base[ERC4626Router].
  *
+ * Since v4.9, this implementation uses virtual assets and shares to mitigate that risk. The `_decimalsOffset()`
+ * corresponds to an offset in the decimal representation between the underlying asset's decimals and the vault
+ * decimals. This offset also determines the rate of virtual shares to virtual assets in the vault, which itself
+ * determines the initial exchange rate. While not fully preventing the attack, analysis shows that the default offset
+ * (0) makes it non-profitable, as a result of the value being captured by the virtual shares (out of the attacker's
+ * donation) matching the attacker's expected gains. With a larger offset, the attack becomes orders of magnitude more
+ * expensive than it is profitable. More details about the underlying math can be found
+ * xref:erc4626.adoc#inflation-attack[here].
+ *
+ * The drawback of this approach is that the virtual shares do capture (a very small) part of the value being accrued
+ * to the vault. Also, if the vault experiences losses, the users try to exit the vault, the virtual shares and assets
+ * will cause the first user to exit to experience reduced losses in detriment to the last users that will experience
+ * bigger losses. Developers willing to revert back to the pre-v4.9 behavior just need to override the
+ * `_convertToShares` and `_convertToAssets` functions.
+ *
+ * To learn more, check out our xref:erc4626.adoc[ERC-4626 guide].
+ * ====
+ *
  * _Available since v4.7._
  */
 abstract contract ERC4626 is ERC20, IERC4626 {
     using Math for uint256;
 
     IERC20 private immutable _asset;
-    uint8 private immutable _decimals;
+    uint8 private immutable _underlyingDecimals;
 
     /**
      * @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC20 or ERC777).
      */
     constructor(IERC20 asset_) {
         (bool success, uint8 assetDecimals) = _tryGetAssetDecimals(asset_);
-        _decimals = success ? assetDecimals : super.decimals();
+        _underlyingDecimals = success ? assetDecimals : 18;
         _asset = asset_;
     }
 
@@ -65,7 +85,7 @@ abstract contract ERC4626 is ERC20, IERC4626 {
      * See {IERC20Metadata-decimals}.
      */
     function decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) {
-        return _decimals;
+        return _underlyingDecimals + _decimalsOffset();
     }
 
     /** @dev See {IERC4626-asset}. */
@@ -90,7 +110,7 @@ abstract contract ERC4626 is ERC20, IERC4626 {
 
     /** @dev See {IERC4626-maxDeposit}. */
     function maxDeposit(address) public view virtual override returns (uint256) {
-        return _isVaultHealthy() ? type(uint256).max : 0;
+        return type(uint256).max;
     }
 
     /** @dev See {IERC4626-maxMint}. */
@@ -179,44 +199,14 @@ abstract contract ERC4626 is ERC20, IERC4626 {
      * would represent an infinite amount of shares.
      */
     function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256) {
-        uint256 supply = totalSupply();
-        return
-            (assets == 0 || supply == 0)
-                ? _initialConvertToShares(assets, rounding)
-                : assets.mulDiv(supply, totalAssets(), rounding);
-    }
-
-    /**
-     * @dev Internal conversion function (from assets to shares) to apply when the vault is empty.
-     *
-     * NOTE: Make sure to keep this function consistent with {_initialConvertToAssets} when overriding it.
-     */
-    function _initialConvertToShares(
-        uint256 assets,
-        Math.Rounding /*rounding*/
-    ) internal view virtual returns (uint256 shares) {
-        return assets;
+        return assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);
     }
 
     /**
      * @dev Internal conversion function (from shares to assets) with support for rounding direction.
      */
     function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256) {
-        uint256 supply = totalSupply();
-        return
-            (supply == 0) ? _initialConvertToAssets(shares, rounding) : shares.mulDiv(totalAssets(), supply, rounding);
-    }
-
-    /**
-     * @dev Internal conversion function (from shares to assets) to apply when the vault is empty.
-     *
-     * NOTE: Make sure to keep this function consistent with {_initialConvertToShares} when overriding it.
-     */
-    function _initialConvertToAssets(
-        uint256 shares,
-        Math.Rounding /*rounding*/
-    ) internal view virtual returns (uint256) {
-        return shares;
+        return shares.mulDiv(totalAssets() + 1, totalSupply() + 10 ** _decimalsOffset(), rounding);
     }
 
     /**
@@ -262,10 +252,7 @@ abstract contract ERC4626 is ERC20, IERC4626 {
         emit Withdraw(caller, receiver, owner, assets, shares);
     }
 
-    /**
-     * @dev Checks if vault is "healthy" in the sense of having assets backing the circulating shares.
-     */
-    function _isVaultHealthy() private view returns (bool) {
-        return totalAssets() > 0 || totalSupply() == 0;
+    function _decimalsOffset() internal view virtual returns (uint8) {
+        return 0;
     }
 }

docs/modules/ROOT/nav.adoc

@@ -13,6 +13,7 @@
 ** xref:erc721.adoc[ERC721]
 ** xref:erc777.adoc[ERC777]
 ** xref:erc1155.adoc[ERC1155]
+** xref:erc4626.adoc[ERC4626]
 
 * xref:governance.adoc[Governance]
 

docs/modules/ROOT/pages/erc4626.adoc

@@ -0,0 +1,184 @@
+= ERC4626
+:stem: latexmath
+
+https://eips.ethereum.org/EIPS/eip-4626[ERC4626] is an extension of xref:erc20.adoc[ERC20] that proposes a standard interface for token vaults. This standard interface can be used by widelly different contracts (include lending markets, aggregators, and intrinsically interest bearing tokens), which brings a number of subtleties. Navigating these potential issues is essential to implementing a compliant and composable token vault.
+
+We prodive a base implementation of ERC4626 that includes a simple vault. This contract is designed in a way that allows developer to easily re-configure the vault's behavior, with minimal overrides, while staying compliant. In this guide, we will discuss some security considerations that affect ERC4626. We will also discuss common customization of the vault.
+
+[[inflation-attack]]
+== Security concern: Inflation attack
+
+=== Visualizing the vault
+
+In exchange for the assets deposited into an ERC4626 vault, a user receives shares. These shares can later be burned to redeem the corresponding underlying assets. The number of shares a user get depend on the amount of assets they put in and on the exchange rate of the vault. This exchange rate is defined by the current liquidity held by the vault.
+
+- If a vault has 100 tokens to back 200 shares, than each share is worth 0.5 asset.
+- If a vault has 200 tokens to back 100 shares, than each share is worth 2.0 asset.
+
+In other words, the exchange rate can be defined as the slope of the line that passes through the origin and the current number of assets and shares in the vault. Deposits and withdrawals move the vault in this line.
+
+image::erc4626-rate-linear.png[Exchange rates in linear scale]
+
+When plotted in log-log scale, the rate is defined similarly, but appears differently (because the point (0,0) is infinitelly far away). Rates are represented by "diagonals" lines with different offsets.
+
+image::erc4626-rate-loglog.png[Exchange rates in logarithmic scale]
+
+In such a reprentation, widelly different rates can be clearly visible in the same graph. This wouldn't be the case in linear scale.
+
+image::erc4626-rate-loglogext.png[More exchange rates in logarithmic scale]
+
+=== The attack
+
+When depositing tokens, the number of shares a user gets is rounded down. This rounding takes away value from the user in favor or the vault (i.e. in favor of all the current share holders). This rounding is often negligeable because of the amount at stake. If you deposit 1e9 shares worth of tokens, the rounding will have you lose at most 0.0000001% of your deposit. However if you deposit 10 shares worth of tokens, you could lose 10% of your deposit. Even worst, if you deposit <1 share worth of tokens, then you get 0 shares, and you basically made a donation.
+
+For a given amount of assets, the more shares you receive the safer you are. If you want to limit your loses to at most 1%, you need to receive at least 100 shares.
+
+image::erc4626-deposit.png[Depositing assets]
+
+In the figure we can see that for a given deposit of 500 asset, the number of share we get, and the corresponding rounding loses depend on the exchange rate. If the exchange rate is that of the orange curve, we are getting less than a share, so we lose 100% of our deposit. However, if the exchange rate is that of the green curve, we get 5000 shares, with limits our rounding loses to at most 0.02%.
+
+image::erc4626-mint.png[Minting shares]
+
+Symmetrically, if we focus on limiting our loses to a maximum of 0.5%, we need to get at least 200 shares. With the green exchange rate that requires just 20 tokens, but with the orange rate that requires 200000 tokens.
+
+We can clearly see that that the blue and green curves correspond to vaults that are safer than the yellow and orange curves.
+
+The idea of an inflation attack is that an attacker can donate assets to the vault to move the rate curve to the right, and make the vault unsafe.
+
+image::erc4626-attack.png[Inflation attack without protection]
+
+Figure 6 shows how an attacker can manipulate the rate of an empty vault. First the attacker must deposit a small amount of tokens (1 token) and follow up with a donation of 1e5 tokens directly to the vault to move the exchange rate "right". This puts the vault in a state where any deposit smaller than 1e5 would be completely lost to the vault. Given that the attacker is the only share holders (from its donation), the attacker would steal all the tokens deposited.
+
+An attacker would typically wait for a user to do the first deposit into the vault, and would frontrun that operation with the attack described above. The risk is low, and the size of the "donation" required to manipulate the vault is equivalent to the size of the deposit that is being attacked.
+
+In math that gives:
+
+- stem:[de] the attacker deposit
+- stem:[do] the attacker donation
+- stem:[du] the user deposit
+
+[%header,cols=4*]
+|===
+|
+| Assets
+| Shares
+| Rate
+
+| initial
+| stem:[0]
+| stem:[0]
+| -
+
+| after attacker's deposit
+| stem:[de]
+| stem:[de]
+| stem:[1]
+
+| after attacker's donation
+| stem:[de+do]
+| stem:[de]
+| stem:[\frac{de}{de+do}]
+|===
+
+This means a deposit of stem:[du] will give stem:[\frac{du*de}{de+do}] shares.
+
+For the attacker to dilute that deposit to 0 shares, causing the user to lose all its deposit, it must ensure that
+
+[stem]
+++++
+\frac{du*de}{de+do} < 1 \iff du < 1 + \frac{do}{de}
+++++
+
+Using stem:[de = 1] and stem:[do = du] is enough. So the attacker only needs stem:[du+1] assets to perform a successful attack.
+
+=== Defending with a virtual offset
+
+The defence we propose consists in two part:
+
+- Use an offset between the "precision" of the representation of shares and assets. Said otherwise, we use more decimals places to represent the shares then the underlying token does to represent the assets.
+- Include virtual shares and virtual assets in the exchange rate computation. These virtuals assets enforce the conversion rate when the vault is empty.
+
+These two part work together in enforcing the security of the vault. First, the increase precision correspond to a high rate, which we saw is safer as it reduces the rounding error when computing the amount of shares. Second, the virtual assets and shares (in addition to simplifying a lot of the computations) capture part of the donation, making it unprofitable for a developer to perform an attack.
+
+
+Following the previous math definition, we have:
+
+- stem:[off] the vault offset
+- stem:[de] the attacker deposit
+- stem:[do] the attacker donation
+- stem:[du] the user deposit
+
+[%header,cols=4*]
+|===
+|
+| Assets
+| Shares
+| Rate
+
+| initial
+| stem:[1]
+| stem:[10^{off}]
+| stem:[10^{off}]
+
+| after attacker's deposit
+| stem:[1+de]
+| stem:[10^{off} \times (1+de)]
+| stem:[10^{off}]
+
+| after attacker's donation
+| stem:[1+de+do]
+| stem:[10^{off} \times (1+de)]
+| stem:[10^{off} \times \frac{1+de}{1+de+do}]
+|===
+
+One important thing to note is that the attacker only owns a fraction stem:[\frac{de}{1+de}] of the shares, so when doing the donation, he will only be able to recover that fraction stem:[\frac{do*de}{1+de}] of the donation. The remaining stem:[\frac{do}{1+de}] are captured by the vault.
+
+[stem]
+++++
+loss = \frac{do}{1+de}
+++++
+
+When the user deposits stem:[du], he receives
+
+[stem]
+++++
+10^{off} \times du \times \frac{1+de}{1+de+do}
+++++
+
+For the attacker to dilute that deposit to 0 shares, causing the user to lose all its deposit, it must ensure that
+
+[stem]
+++++
+10^{off} \times du \times \frac{1+de}{1+de+do} < 1
+++++
+
+[stem]
+++++
+\iff 10^{off} \times du < \frac{1+de+do}{1+de}
+++++
+
+[stem]
+++++
+\iff 10^{off} \times du < 1 + \frac{do}{1+de}
+++++
+
+[stem]
+++++
+\iff 10^{off} \times du \le loss
+++++
+
+- If the offset is 0, the attacker loss is at least equal to the users deposit.
+- If the offset is greater than 0, the attacker will have to suffer losses that are orders of magnitude bigger than the amount of value that can hypothetically be stolen from the user.
+
+This shows that even with an offset of 0, the virtual shares and assets make this attack non profitable for the attacker. Bigger offsets increase the security even further by making any attack on the user extremely wasteful.
+
+The following figure show how the offset impacts the initial rate and limits the ability of an attacker with limited fund to inflate it effectivelly.
+
+image::erc4626-attack-3a.png[Inflation attack without offset=3]
+stem:[off = 3], stem:[de = 1], stem:[do = 10^5]
+
+image::erc4626-attack-3b.png[Inflation attack without offset=3 and an attacker deposit that limits its loses]
+stem:[off = 3], stem:[de = 100], stem:[do = 10^5]
+
+image::erc4626-attack-6.png[Inflation attack without offset=6]
+stem:[off = 6], stem:[de = 1], stem:[do = 10^5]