sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

shaka - Use of wrong collateral price #215

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 8 months ago

shaka

high

Use of wrong collateral price

Summary

As rETH is the collateral token, its price is supposed to be used for calculations. However, the price used in the protocol is the ETH price.

Vulnerability Detail

The protocol uses rETH as collateral and thus the OracleModule.getPrice contract should return the price of rETH. This is confirmed in different places in the documentation.

From README.md:

Q: Are the admins of the protocols your contracts integrate with (if any) TRUSTED or RESTRICTED? TRUSTED: collateral asset will be rETH, Pyth Network oracle for rETH

Q: Are there any off-chain mechanisms or off-chain procedures for the protocol (keeper bots, input validation expectations, etc)? The protocol uses Pyth Network collateral (rETH) price feed.

Q: In case of external protocol integrations, are the risks of external contracts pausing or executing an emergency withdrawal acceptable? If not, Watsons will submit issues related to these situations that can harm your protocol's functionality. Only rETH and Pyth rETH oracle dependency.

From flatcoin-v1/README.md:

The oracle can provide the LSD price via Pyth network and Chainlink oracles for the most up-to-date prices. Chainlink is only used as a sanity check in case Pyth Oracle reports inaccurate prices.

From website docs:

The Flat Money protocol uses Pyth Network to get an accurate rETH price oracle. The oracle ensures that all the UNIT and leveraged rETH positions are priced correctly. For each order, the user announces their order and after a short delay, a keeper executes the order with a fresh rETH price from Pyth.

We can also find many references in the PerpMath contract that refer to the price used in the calculations:

/// @return pnl The PnL in terms of the market currency (ETH/LST) and not in dollars ($).

However, in the test suite, WETH is used as collateral and thus, the ETH price is retrieved from the oracles.

The deployment script configuration also suggests that the ETH price is used.

This discrepancy is likely to be a result of the protocol having been developed to be used with ETH as collateral and later changed to rETH.

There are two possible interpretations for the price that will be used in the protocol:

1) The rETH price feeds will be used instead of the ETH price feeds we see in the tests and deployment scripts.

However, there is currently no RETH/USD price feed for Chainlink available in the Base network, so the rETH price feeds cannot be used.

2) The ETH price feeds will be used.

In this case, the collateral will be priced at the wrong value as ETH and rETH are not 1:1 pegged.

Impact

All calculations for PnL, liquidation, funding rate, etc. will be incorrect and cause users to receive the wrong amount of funds and be liquidated at the wrong price.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L138-L157

Tool used

Manual Review

Recommendation

Adapt OracleModule:_getOnchainPrice to fetch both the RETH/ETH and ETH/USD prices and calculate the RETH/USD price, as explained in the Chainlink documentation.

Take special attention to the number of decimals expected from the price feeds. The current implementation is assuming to receive the price with 8 decimals, which is correct for the ETH/USD price feed, but not for the RETH/ETH price feed, which has 18 decimals.

Duplicate of #90

sherlock-admin commented 7 months ago

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

takarez commented:

invalid: