code-423n4 / 2023-03-asymmetry-findings

14 stars 12 forks source link

Fund stealing via Donation Attack on `SafEth.stake()` #1063

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L98-L99

Vulnerability details

Impact

By simple manipulations, the first depositor can infinitely drain all next depositors' funds by causing a rounding error in SafEth.stake() share-issuing logic and forcing it to mint zero SafEth shares to all next stakers.

A similar vulnerability was highlighted in OpenZeppelin ERC4626 audit (HIGH-01): https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/audits/2022-10-ERC4626.pdf

To attack SafEth a hacker needs to become the first investor with any amount of staked ETH, then withdraw the most of the stake so there is only 1 wei of SafEth shares left, and then inflate the underlying asset value by manually donating a large amount of underlying assets to relevant derivative pools. Any staker after the hacker's attack will receive zero shares for their stake. The attacker can redeem all underlying funds at any time by burning the only 1 wei share in the contract.

Proof of Concept

Attack scenario:

Step 1. A hacker backruns SafEth.initialize() function and becomes the first investor in the contract by staking minAmount ETH via SafEth.stake().

The contract is initialized with minAmount = 0.5 ETH:

minAmount = 5 * 10 ** 17; // initializing with .5 ETH as minimum

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L54

Right after initialization the SafEth.totalSupply is zero. Thus, the contract mints 0.5 SafEth shares to the hacker:

if (totalSupply == 0)
    preDepositPrice = 10 ** 18; // initializes with a price of 1

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L79-L80

Step 2. The hacker unstakes most of the funds via SafEth.unstake() so there is only 1 single wei share on the balance left. This can be done by calling:

safEth.unstake(safEth.balanceOf(hacker) - 1)

Now the SafEth.totalSupply is 1 wei.

Step 3. The hacker inflates the underlying asset value.

For example, the hacker buys 300.0 WstEth from the market and directly donates it to the project's WstEth derivative pool by calling

IERC20(WST_ETH).transfer(address(derivativeContract), 300e18)

The WstEth derivative contract's balance is calculated as follows:

function balance() public view returns (uint256) {
    return IERC20(WST_ETH).balanceOf(address(this));
}

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/WstEth.sol#L93-L96

After the hacker's direct transfer it returns a value greater or equal to 300e18.

Note that the hacker can redeem the full sum (300.0 ETH) at any time since they own 100% of SafEth shares (even though it's just a 1 wei).

Step 4. . Any new depositor that stakes funds after the hacker will receive zero SafEth shares because of a rounding error in the SafEth.stake() function.

To make a point we'll consider a scenario in which a victim stakes 200.0 ETH.

Note that after the contract's initialization the maxAmount that any user can deposit via stake() function is constrained by 200 ETH, so no one can stake more than this amount:

maxAmount = 200 * 10 ** 18; // initializing with 200 ETH as maximum

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L55

When the victim calls stake(), the function first checks the requirements which are all passed:

require(pauseStaking == false, "staking is paused");
require(msg.value >= minAmount, "amount too low");
require(msg.value <= maxAmount, "amount too high");

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L64-L66

Then the contract calculates the underlying asset value:

uint256 underlyingValue = 0;

// Getting underlying value in terms of ETH for each derivative
for (uint i = 0; i < derivativeCount; i++)
    underlyingValue +=
        (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
            derivatives[i].balance()) /
        10 ** 18;

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L68-L77

Note that the hacker directly donated 300.0 ETH to the WstEth contract, so the underlyingValue is equal to 300e18.

Next, the preDepositPrice is calculated:

uint256 totalSupply = totalSupply();
uint256 preDepositPrice; // Price of safETH in regards to ETH
if (totalSupply == 0)
    preDepositPrice = 10 ** 18; // initializes with a price of 1
else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L77-L82

Note that the totalSupply is still equal to 1 wei, and underlyingValue is equal to 300e18, so preDepositPrice is calculated as 10 ** 18 * 300e18 / 1 == 1e18 * 300e18.

The following code distributes the victim's funds to different derivatives and calculates totalStakeValueEth:

uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
for (uint i = 0; i < derivativeCount; i++) {
    uint256 weight = weights[i];
    IDerivative derivative = derivatives[i];
    if (weight == 0) continue;
    uint256 ethAmount = (msg.value * weight) / totalWeight;

    // This is slightly less than ethAmount because slippage
    uint256 depositAmount = derivative.deposit{value: ethAmount}();
    uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
        depositAmount
    ) * depositAmount) / 10 ** 18;
    totalStakeValueEth += derivativeReceivedEthValue;
}

Note that the totalStakeValueEth is expected to return a value close to the user's deposit, so, in our case, it can be considered equal to the 200e18.

Now the mintAmount is calculated:

// mintAmount represents a percentage of the total assets in the system
uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

The mintAmount is calculated as 200e18 * 1e18 / (1e18 * 300e18) which is equal to zero.

Note that there is no requirement that the minAmount cannot be zero. Thus, the victim receives zero shares:

_mint(msg.sender, mintAmount);

Step 5. The hacker redeems all the funds.

Note that any new depositor funds are drained by the hacker since no one can stake more than 200.0 ETH. And even if admins increase the threshold, the hacker can simply increase the donation above the threshold, fronrunning any large stake.

When the hacker calls SafEth.unstake(1), the derivativeAmount to be withdrawn is calculated as follows:

// withdraw a percentage of each asset based on the amount of safETH
uint256 derivativeAmount = (derivatives[i].balance() *
    _safEthAmount) / safEthTotalSupply;
if (derivativeAmount == 0) continue; // if derivative empty ignore
derivatives[i].withdraw(derivativeAmount);

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L114-L118

Note that safEthTotalSupply and _safEthAmount are equal to 1. Thus, the derivativeAmount is equal to the derivatives[i].balance(). So the hacker can withdraw all the funds from all the derivatives.

Tools Used

x

Recommended Mitigation Steps

One way to resolve the problem is to mint 1000 dead wei shares to zero address in the initialize() function.

Other ways are descibed in the following thread:

Also take a note of a great article which analyzes flaws of different approaches of how to protect against different kinds of Inflation Attacks:

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #715

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory