xiaoming90 - Stat calculator returns incorrect report for swETH

[sherlock-admin2]

xiaoming90

high

Stat calculator returns incorrect report for swETH

Summary

Stat calculator returns incorrect reports for swETH, causing multiple implications that could lead to losses to the protocol,

Vulnerability Detail

The purpose of the in-scope SwEthEthOracle contract is to act as a price oracle specifically for swETH (Swell ETH) per the comment in the contract below and the codebase's README

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/oracles/providers/SwEthEthOracle.sol#L16

File: SwEthEthOracle.sol
12: /**
13:  * @notice Price oracle specifically for swEth (Swell Eth).
14:  * @dev getPriceEth is not a view fn to support reentrancy checks. Does not actually change state.
15:  */
16: contract SwEthEthOracle is SystemComponent, IPriceOracle {

Per the codebase in the contest repository, the price oracle for the swETH is understood to be configured to the SwEthEthOracle contract at Line 252 below.

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/test/oracles/RootOracleIntegrationTest.t.sol#L252

File: RootOracleIntegrationTest.t.sol
162:         swEthOracle = new SwEthEthOracle(systemRegistry, IswETH(SWETH_MAINNET));
..SNIP..
249:         // Lst special pricing case setup
250:         // priceOracle.registerMapping(SFRXETH_MAINNET, IPriceOracle(address(sfrxEthOracle)));
251:         priceOracle.registerMapping(WSTETH_MAINNET, IPriceOracle(address(wstEthOracle)));
252:         priceOracle.registerMapping(SWETH_MAINNET, IPriceOracle(address(swEthOracle)));

Thus, in the context of this audit, the price oracle for the swETH is mapped to the SwEthEthOracle contract.

Both the swETH oracle and calculator use the same built-in swEth.swETHToETHRate function to retrieve the price of swETH in ETH.

LST	Oracle	Calculator	Rebasing
swETH	SwEthEthOracle - swEth.swETHToETHRate()	SwethLSTCalculator - IswETH(lstTokenAddress).swETHToETHRate()	False

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/oracles/providers/SwEthEthOracle.sol#L26

File: SwEthEthOracle.sol
25:     /// @inheritdoc IPriceOracle
26:     function getPriceInEth(address token) external view returns (uint256 price) {
..SNIP..
30:         // Returns in 1e18 precision.
31:         price = swEth.swETHToETHRate();
32:     }

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/stats/calculators/SwethLSTCalculator.sol#L12

File: SwethLSTCalculator.sol
12:     function calculateEthPerToken() public view override returns (uint256) {
13:         return IswETH(lstTokenAddress).swETHToETHRate();
14:     }

Within the LSTCalculatorBase.current function, assume that the swEth.swETHToETHRate function returns $x$ when called. In this case, the price at Line 203 below and backing in Line 210 below will be set to $x$ since the getPriceInEth and calculateEthPerToken functions depend on the same swEth.swETHToETHRate function internally. Thus, priceToBacking will always be 1e18:

$$ \begin{align} priceToBacking &= \frac{price \times 1e18}{backing} \\ &= \frac{x \times 1e18}{x} \\ &= 1e18 \end{align} $$

Since priceToBacking is always 1e18, the premium will always be zero:

$$ \begin{align} premium &= priceToBacking - 1e18 \\ &= 1e18 - 1e18 \\ &= 0 \end{align} $$

As a result, the calculator for swETH will always report the wrong statistic report for swETH. If there is a premium or discount, the calculator will wrongly report none.

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/stats/calculators/base/LSTCalculatorBase.sol#L189

File: LSTCalculatorBase.sol
189:     function current() external returns (LSTStatsData memory) {
..SNIP..
202:         IRootPriceOracle pricer = systemRegistry.rootPriceOracle();
203:         uint256 price = pricer.getPriceInEth(lstTokenAddress);
..SNIP..
210:             uint256 backing = calculateEthPerToken();
211:             // price is always 1e18 and backing is in eth, which is 1e18
212:             priceToBacking = price * 1e18 / backing;
213:         }
214: 
215:         // positive value is a premium; negative value is a discount
216:         int256 premium = int256(priceToBacking) - 1e18;
217: 
218:         return LSTStatsData({
219:             lastSnapshotTimestamp: lastSnapshotTimestamp,
220:             baseApr: baseApr,
221:             premium: premium,
222:             slashingCosts: slashingCosts,
223:             slashingTimestamps: slashingTimestamps
224:         });
225:     }

Impact

The purpose of the stats/calculators contracts is to store, augment, and clean data relevant to the LMPs. When the solver proposes a rebalance, the strategy uses the stats contracts to calculate a composite return (score) for the proposed destinations. Using that composite return, it determines if the swap is beneficial for the vault.

If a stat calculator provides inaccurate information, it can cause multiple implications that lead to losses to the protocol, such as false signals allowing the unprofitable rebalance to be executed.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/stats/calculators/base/LSTCalculatorBase.sol#L189

Tool used

Manual Review

Recommendation

When handling the swETH within the LSTCalculatorBase.current function, consider other methods of obtaining the fair market price of swETH that do not rely on the swEth.swETHToETHRate function such as external 3rd-party price oracle.

[sherlock-admin2]

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

Trumpero commented:

invalid, I think it's intended to let premium = 0 in case of swETH. Also I don't see any further vulnerability if premium = 0

[xiaoming9090]

Escalate

After my discussion with the protocol team during the audit period (as shown below), the purpose of the premium variable is to determine if an LST is trading at a premium/discount relative to its actual ETH backing. This is one of the signals the protocol team uses to estimate the expected return for holding an LST position.

It is incorrect that the premium for swETH is intended to be always zero, as mentioned in the judging comment. If the premium always returns zero, the stat calculator for swETH is effectively broken, which is a serious issue. The judging comment is also incorrect in stating there is no vulnerability if the premium is zero. The stat calculator exists for a reason, and an incorrect stat calculator ultimately leads to losses to the protocol, as mentioned in the "Impact" section of my report:

The purpose of the stats/calculators contracts is to store, augment, and clean data relevant to the LMPs. When the solver proposes a rebalance, the strategy uses the stats contracts to calculate a composite return (score) for the proposed destinations. Using that composite return, it determines if the swap is beneficial for the vault.

If a stat calculator provides inaccurate information, it can cause multiple implications that lead to losses to the protocol, such as false signals allowing the unprofitable rebalance to be executed.

Thus, this is a valid High issue.

[sherlock-admin2]

Escalate

After my discussion with the protocol team during the audit period (as shown below), the purpose of the premium variable is to determine if an LST is trading at a premium/discount relative to its actual ETH backing. This is one of the signals the protocol team uses to estimate the expected return for holding an LST position.

It is incorrect that the premium for swETH is intended to be always zero, as mentioned in the judging comment. If the premium always returns zero, the stat calculator for swETH is effectively broken, which is a serious issue. The judging comment is also incorrect in stating there is no vulnerability if the premium is zero. The stat calculator exists for a reason, and an incorrect stat calculator ultimately leads to losses to the protocol, as mentioned in the "Impact" section of my report:

The purpose of the stats/calculators contracts is to store, augment, and clean data relevant to the LMPs. When the solver proposes a rebalance, the strategy uses the stats contracts to calculate a composite return (score) for the proposed destinations. Using that composite return, it determines if the swap is beneficial for the vault.

If a stat calculator provides inaccurate information, it can cause multiple implications that lead to losses to the protocol, such as false signals allowing the unprofitable rebalance to be executed.

Thus, this is a valid High issue.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Trumpero]

Can u take a look at this issue @codenutt ?

[codenutt]

Can u take a look at this issue @codenutt ?

Yup definitely an issue. Its more an issue with the oracle itself than the calculator, but an issue none the less.

[Evert0x]

Planning to accept escalation and make issue high.

[Evert0x]

Result:
High
Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• xiaoming9090: accepted