Active Boosts

#35876 [SC-High] Users will lose funds on calls to critical functions if the prices are not updated

Submitted on Oct 11th 2024 at 15:51:15 UTC by @SeveritySquad for IOP | Swaylend

  • Report ID: #35876

  • Report Type: Smart Contract

  • Report severity: High

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

  • Impacts:

    • Permanent freezing of funds

Description

Brief/Intro

The internal `update_price_feeds_if_necessary_internal()` transfers the fees for updating the Pyth price-feeds to the oracle, however, the fees will be left stuck in Pyth in scenarios where the prices were never even updated.

Vulnerability Details

The `update_price_feeds_if_necessary_internal()` is called by:

  • `absorb()`.

  • `withdraw_base()`.

  • `withdraw_collateral()`.

Using `withdraw_collateral()` as an example, on the call to it, ```rust #[payable, storage(write)] fn withdraw_collateral( // some code // Update price data //@audit here the price is updated if neccessary. update_price_feeds_if_necessary_internal(price_data_update);

// code for logging } ``` internally the function calls `update_price_feeds_if_necessary_internal()` which further makes a call to Pyth's `update_price_feeds_if_necessary()` function, before the call to Pyth's oracle it ensures that the user sent the amount of fee for the price feeds to be updated and forwards that fee on the call to Pyth's oracle `update_price_feeds_if_necessary()`.

  • https://github.com/Swaylend/swaylend-monorepo/blob/9132747331188b86dd8cbf9a1ca37b811d08dddb/contracts/market/src/main.sw#L1441C1-L1469C2 ```rust #[payable, storage(read)] fn update_price_feeds_if_necessary_internal(price_data_update: PriceDataUpdate) { // some code // check if the payment is sufficient require( msg_amount() >= price_data_update .update_fee && msg_asset_id() == AssetId::base(), Error::InvalidPayment, );

    let oracle = abi(PythCore, contract_id.bits()); oracle .update_price_feeds_if_necessary { asset_id: AssetId::base().bits(), coins: price_data_update.update_fee, }( price_data_update .price_feed_ids, price_data_update .publish_times, price_data_update .update_data, ); }

``` The issue here lies in the fact that Pyth's oracle `update_price_feeds_if_necessary()` which only calls `update_price_feeds()` if the latest publish time is less than the input published time, and fees are only required in `update_price_feeds()` whenever a price feed is updated.

  • https://github.com/pyth-network/pyth-crosschain/blob/9c761626440da0c731d5e41e9b6d31aa5e909bf3/target_chains/fuel/contracts/pyth-contract/src/main.sw#L292C1-L300C10 ```rust while i < price_feed_ids.len() { if latest_publish_time(price_feed_ids.get(i).unwrap()) < publish_times.get(i).unwrap() { update_price_feeds(update_data); return; }

```

  • https://github.com/pyth-network/pyth-crosschain/blob/9c761626440da0c731d5e41e9b6d31aa5e909bf3/target_chains/fuel/contracts/pyth-contract/src/main.sw#L420C2-L421C71

```rust #[storage(read, write), payable] fn update_price_feeds(update_data: Vec<Bytes>) { // code for price feed updates. let required_fee = total_fee(total_number_of_updates, storage.single_update_fee); require(msg_amount() >= required_fee, PythError::InsufficientFee); ```

Impact Details

In scenarios when the price feeds are not updated in Pyth's oracle, the fees for updating those feeds will be left stuck in Pyth with no way to retrieve it, and given the number of transactions and collaterals that would be used the amount the users will lose for price feeds that were never updated will increase exponentially. Also, Pyth fee can increase in the future causing the costs to rise, and concerning this

  • https://github.com/Swaylend/swaylend-monorepo/issues/158

Mitigation

Use the Pyth's `PythInfo::latest_publish_time()` -> https://github.com/pyth-network/pyth-crosschain/blob/9c761626440da0c731d5e41e9b6d31aa5e909bf3/target_chains/fuel/contracts/pyth-contract/src/main.sw#L563C8-L563C27 to get the latest publish times, possibly looping through to check the price feeds that need not be updated and only forwarding to the Pyth oracle the amount of fees required for the feeds that will be updated.

Proof of Concept

Proof of Concept

```bash cargo test local_tests::scenarios::multicall_withdraw_supply::multicall_withdraw_supply_test ``` We update the price feeds, before with call `withdraw_base()` with that same price feed update data, but the fee is still lost to the Pyth oracle ```rust use crate::utils::{print_case_title, setup, TestData}; use fuels::{ accounts::ViewOnlyAccount, programs::{ calls::{CallHandler, CallParameters}, responses::CallResponse, }, types::{transaction::TxPolicies, transaction_builders::VariableOutputPolicy}, }; use market::PriceDataUpdate; use market_sdk::parse_units;

const AMOUNT_COEFFICIENT: u64 = 10u64.pow(0); const SCALE_6: f64 = 10u64.pow(6) as f64; const SCALE_9: f64 = 10u64.pow(9) as f64;

#[tokio::test] async fn multicall_withdraw_supply_test() { let TestData { wallets, alice, alice_account, bob, bob_account, market, usdc, usdc_contract, eth, oracle, price_feed_ids, publish_time, prices, assets, .. } = setup().await;

}

```

Previous#35767 [SC-Critical] constanct value is used to check `price.confidence`Next#35793 [SC-High] `src-20.burn` should use "==" instead of ">="

Last updated

Was this helpful?