Asymmetry contest - Cryptor's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L221-L223 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L71-L75

Vulnerability details

Impact

The function stake uses the balances of each derivative contract to calculate underlyingvalue which is then used to calculate the amount of SafEth to mint to a user. However, this makes the contract vulnerable to an inflation attack where a user can manipulate the mint function by making a direct donation to any and all of the derivative contracts, making the ETH price of SafEth more than what it should be.

Proof of Concept

The function stake calculates the variable underlying value with the following formula

            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#L71-L75

This variable is used to calculate the amount of Safeth to mint to a user

else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

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

and here

uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

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

So when preDepositPrice rises, then the amount to mint to a user goes down.

The function to get the balances in the each derivative is shown here which is the same across all 3 current derivatives:

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

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

So a malicious user can direct deposit Reth, SfrxEth, or WstEth into the contract to inflate underlying value and forcibly reduce the amount of SafEth to mint. For example Bob makes a malicious donation to the derivative contracts. Alice calls stake after depositing some ETH and expecting some SafETH back. However, Alice receives 0 SafETH because Bob has inflated predepositprice. Bob can then make his own larger deposit and steal Alice's rewards by calling unstake.

Tools Used

Manual Review

Recommended Mitigation Steps

Use a mapping instead of balanceOf to track balances

Lines of code

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

Vulnerability details

Impact

The function withdraw in Reth.sol uses the burn function in the Reth contract, in order to swap Reth for ETH.

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

However, theReth contract does not have a lot of liquidity. As of this writing the amount of ETH in the Reth contract is only 536 ETH as shown here

https://etherscan.io/address/0xae78736Cd615f374D3085123A210448E74Fc6393

And the totalsupply of Reth is 216546.916776976708709502 in ETH

In addition, the price of Reth is $1,894.35 as of this writing. In the event that the Reth contract is compromised, there will not be enough liquidity in the contract to cover all users meaning that the function withdraw will revert.

Proof of Concept

See Impact

Tools Used

Manual Review

Recommended Mitigation Steps

Just like the deposit function, add a check to swap Reth for ETH in uniswap if the burn function from the Reth contract fails.

I am listing this as a medium because user funds will be lost if the Reth contract is compromised

Lines of code

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

Vulnerability details

Impact

The function stake uses a for loop to cycle through all of the derivatives and their respective weights in order to calculate the amount to deposit into each derivative. However, neither the derivatives or their weights are defined in the initialize function. That means that if the admin does not call rebalanceToWeights function or the adjustweight function before a user calls the function stake, the function stake will not work properly for users due to a miscalculation in the variable ethamount.

Proof of Concept

The function stake calculates the amount of ETH to deposit into each derivative contract like this

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

However, since the derivatives and their weights are not called, ethamount will return 0. Reth derivative contract has a minimum deposit of 0.01, making the entire stake function revert.

Tools Used

Manual Review

Recommended Mitigation Steps

In the initialize function, set the derivatives and their respective weights. Alternatively, pause both stake and unstake in the initialize function until the derivatives and weights have been set.

No address 0 check in function addDerivative

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

No restriction on the weight of a derivative

Weight can be any uint. This can cause some issues if weight is set too high

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

Precision loss in ethamount if the weights are denominated too high (e.g 10**18)

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

If totalweights is too high (e.g. 1000 * (10**18)), then ethamount will revert in some situations

Recommendation: Make sure that weight are not denominated too high

Users do not have time to react if rebalance weights is called

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

Recommendation: Give users lag time to withdraw if they do not agree with the weight rebalance