#35750 [SC-High] User loss due to Pyth oracle update fee being smaller than the msg amount sent

Submitted on Oct 6th 2024 at 13:02:39 UTC by @SimaoAmaro for IOP | Swaylend

• Report ID: #35750

• Report Type: Smart Contract

• Report severity: High

• Target: https://github.com/Swaylend/swaylend-monorepo/blob/develop/contracts/market/src/main.sw

• Impacts:

  • Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

Description

Brief/Intro

The Swaylend market allows users to send price updates directly when interacting with several functions.

The Pyth oracle requires paying a fee whenever paying prices, but it does not always charge the same fee and it may not even charge any fee.

However, the market contract does not deal with the 2 scenarios above and will lead to user loss of funds.

Vulnerability Details

The `market::update_price_feeds_if_necessary_internal()` function forwards the `msg_amount` sent in the the `price_data_update` argument, without confirming if this fee is the right one. Then, it forwards the fee to the pyth oracle, which might be more than necessary [1].

However, there are 2 situations that might lead to loss of funds for the user.

1. In case the user is frontrun by an update of the pyth price, the latest submitted publish time of the oracle will be more recent than the update data the user sent as argument to the market function. When this happens, the pyth oracle ignores the update data and does not enforce any payment, leading to loss of funds for the users that could have been saved. [2]

2. In case the pyth oracle updates the fee and makes it smaller and frontruns the user. This means the user will send more fee than it should and it will never revert.

Impact Details

Users will overpay for pyth oracle fees not getting these funds back, incurring loss of funds.

References

[1] https://github.com/Swaylend/swaylend-monorepo/blob/develop/contracts/market/src/main.sw?utm_source=immunefi#L1067 [2] https://github.com/FuelLabs/Pyth-integration/blob/master/pyth-contract/src/main.sw#L273

Proof of Concept

Proof of Concept

Apply the following diffs to confirm that it may overcharge fees. ```diff diff --git a/contracts/market/tests/local_tests/main_test_uni_no_debug_mode.rs b/contracts/market/tests/local_tests/main_test_uni_no_debug_mode.r s index 6976dc8..9b4ff8b 100644 --- a/contracts/market/tests/local_tests/main_test_uni_no_debug_mode.rs +++ b/contracts/market/tests/local_tests/main_test_uni_no_debug_mode.rs @@ -3,7 +3,7 @@ use chrono::Utc; use fuels::prelude::ViewOnlyAccount; use fuels::programs::calls::{CallHandler, CallParameters}; use fuels::programs::responses::CallResponse; -use fuels::types::transaction::TxPolicies; +use fuels::types::{transaction::TxPolicies, AssetId}; use fuels::types::transaction_builders::VariableOutputPolicy; use market::PriceDataUpdate; use market_sdk::{convert_i256_to_u64, is_i256_negative, parse_units}; @@ -575,6 +575,8 @@ async fn main_test_no_debug() { let log_amount = format!("{} UNI", amount as f64 / scale_9); print_case_title(12, "Chad", "withdraw_collateral", log_amount.as_str());

• let base_balance_before = chad.get_asset_balance(&AssetId::zeroed()).await.unwrap();

• // Chad calls withdraw_collateral market .with_account(&chad) @@ -588,6 +590,8 @@ async fn main_test_no_debug() { ) .await .unwrap();

• assert!(chad.get_asset_balance(&AssetId::zeroed()).await.unwrap() + 102 == base_balance_before);

diff --git a/libs/market_sdk/src/market_utils.rs b/libs/market_sdk/src/market_utils.rs index a9f8592..0433364 100644 --- a/libs/market_sdk/src/market_utils.rs +++ b/libs/market_sdk/src/market_utils.rs @@ -274,7 +274,7 @@ impl Market { price_data_update: &PriceDataUpdate, ) -> anyhow::Result<CallResponse<()>> { let tx_policies = TxPolicies::default().with_script_gas_limit(DEFAULT_GAS_LIMIT);

•    let call_params = CallParameters::default().with_amount(price_data_update.update_fee);

•    let call_params = CallParameters::default().with_amount(price_data_update.update_fee + 100);

```