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

14 stars 12 forks source link

Derivate `WstEth` contract incorrectly calculates the price of the `ETH` #648

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/derivatives/WstEth.sol#L87

Vulnerability details

Impact

The WstEth contract incorrectly calculates the price of ETH in terms of WstETH.

The function returns the price of WstETH in terms of stETH. The underlying asset which we desire is ETH.

Since stETH does not have the same value as ETH the output price is incorrect.

The impact is severe as underlyingValue and totalStakeValueEth through the stake() function in the SafEth.sol contract SafEth.sol#L63 are wrong calculated.

Proof of Concept

The function IWstETH(token).getStETHByWstETH() only converts WstETH to stETH. Thus, ethPerDerivative() returns the value of WstETH in terms of stETH. Also, the @notice comment in the code says that the derivate's price should be in terms of ETH.

    /**
        @notice - Get price of derivative in terms of ETH
     */
    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
    }

Tools Used

Manuel review https://code4rena.com/reports/2022-05-alchemix/#m-15-lido-adapter-incorrectly-calculates-the-price-of-the-underlying-token

Recommended Mitigation Steps

Add extra steps to ethPerDerivative() to approximate the rate for converting stETH to ETH. This can be done using the same curve pool that is used to convert stETH to ETH in withdraw().

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #588

c4-judge commented 1 year ago

Picodes changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory