sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

LTDingZhen - ERC4626 Vault Inflation Attack #125

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

LTDingZhen

medium

ERC4626 Vault Inflation Attack

Summary

Malicious users can perform an inflation attack against the LSTAdapter to steal the assets of the victim.

Vulnerability Detail

A malicious user can perform a donation to execute a classic first depositor/ERC4626 inflation Attack against the LSTAdapter.(For simplicity, we only consider StEtherAdapter below). The general process of this attack is well-known, and a detailed explanation of this attack can be found in many of the resources such as the following:

In short, to kick-start the attack, the malicious user will often usually mint the smallest possible amount of shares (e.g., 1 wei) and then donate significant assets to the vault to inflate the number of assets per share. Subsequently, it will cause a rounding error when other users deposit.

However, in Napier, OZ's ERC4626 implementation is used, which has various safeguards in place to mitigate this attack: Virtual Shares and Decimals Offset.

It 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.

However, as OZ's _decimalsOffset is set to 0 by default, and current BaseLSTAdapter does not override the function, there is actually no offset:

function _decimalsOffset() internal view virtual returns (uint8) {
    return 0;
}

Although virtual shares and virtual assets could enforce the conversion rate when the vault is empty, such attack could be done by mint x share -> burn x-1 share.

function totalAssets() public view override returns (uint256) {
    uint256 stEthBalance = STETH.balanceOf(address(this));
    return withdrawalQueueEth + bufferEth + stEthBalance;
}

Part 1 - Malicious user mint 1 mint of share

Part 2 - Donate or transfer assets to the vault to inflate the assets per share

Impact

Malicous users could steal the assets of the victim.

Code Snippet

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/8b4b7b8d041c62a84e2c23d7f6e1f0d9e0fc1f20/contracts/token/ERC20/extensions/ERC4626.sol

https://github.com/sherlock-audit/2024-01-napier/blob/6313f34110b0d12677b389f0ecb3197038211e12/napier-v1/src/adapters/BaseLSTAdapter.sol#L71

Tool used

Manual Review

Recommendation

A MIN_LIQUIDITY amount of shares needs to exist within the vault to guard against a common inflation attack.

Or, just use a _decimalsOffset > 0 to forbid the attack.

Duplicate of #94

sherlock-admin commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(2)