Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC4626 inflation attack mitigation #3979

Merged
merged 27 commits into from
Feb 17, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/perfect-insects-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`ERC4626`: Add mitigation to the inflation attack through virtual shares and assets.
33 changes: 0 additions & 33 deletions contracts/mocks/token/ERC4626DecimalsMock.sol

This file was deleted.

17 changes: 17 additions & 0 deletions contracts/mocks/token/ERC4626OffsetMock.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}
73 changes: 30 additions & 43 deletions contracts/token/ERC20/extensions/ERC4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* 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
frangio marked this conversation as resolved.
Show resolved Hide resolved
* 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:ROOT: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_;
}

Expand All @@ -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}. */
Expand All @@ -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}. */
Expand Down Expand Up @@ -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);
Amxx marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @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);
}

/**
Expand Down Expand Up @@ -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;
}
}
Binary file added docs/modules/ROOT/images/erc4626-attack-3a.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-attack-3b.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-attack-6.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-attack.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-deposit.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-mint.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-rate-linear.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/modules/ROOT/images/erc4626-rate-loglog.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions docs/modules/ROOT/nav.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
** xref:erc721.adoc[ERC721]
** xref:erc777.adoc[ERC777]
** xref:erc1155.adoc[ERC1155]
** xref:erc4626.adoc[ERC4626]

* xref:governance.adoc[Governance]

Expand Down
184 changes: 184 additions & 0 deletions docs/modules/ROOT/pages/erc4626.adoc
Original file line number Diff line number Diff line change
@@ -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 widely different contracts (including 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 provide a base implementation of ERC4626 that includes a simple vault. This contract is designed in a way that allows developers 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 customizations 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 gets depends 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, then each share is worth 0.5 assets.
- If a vault has 200 tokens to back 100 shares, then each share is worth 2.0 assets.

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 infinitely far away). Rates are represented by "diagonal" lines with different offsets.

image::erc4626-rate-loglog.png[Exchange rates in logarithmic scale]

In such a reprentation, widely 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 negligible 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 worse, 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 losses 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 assets, the number of shares we get and the corresponding rounding losses 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, which limits our rounding losses to at most 0.02%.

image::erc4626-mint.png[Minting shares]

Symmetrically, if we focus on limiting our losses 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 holder (from their 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
frangio marked this conversation as resolved.
Show resolved Hide resolved

[%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
frangio marked this conversation as resolved.
Show resolved Hide resolved

[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 defense we propose consists of two parts:

- Use an offset between the "precision" of the representation of shares and assets. Said otherwise, we use more decimal places to represent the shares than the underlying token does to represent the assets.
- Include virtual shares and virtual assets in the exchange rate computation. These virtual assets enforce the conversion rate when the vault is empty.

These two parts work together in enforcing the security of the vault. First, the increased precision corresponds 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 definitions, 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 user's 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 shows how the offset impacts the initial rate and limits the ability of an attacker with limited funds to inflate it effectively.

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 losses]
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]