hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

`set_gas_price_oracle()` should be called when the contract is halted #50

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1675daf673d896b024257e304f99b1038f9f7ccb1773252f23ae98265b6e7342 Severity: low

Description: Description\

In ink! Most contract, set_gas_price_oracle() function is called by owner of contract to set the gas price oracle. gas price oracle is an external contract which is basically used to get the ETH gas price in Azero so that base fee can be calculated.

Following functions are called only when the ink! Most contract is halted means when the contract is paused.

1) add_pair() 2) emove_pair() 3) set_committee()

These functions are halted due to removal of pair or addition of pair and upgradation in commitee can affect the ongoing working of send_request and receive_request functions. Thats the reason, both of these functions are called only when the contract is not halted.

The issue here is that when setting the gas price oracle, the contract is not halted and due to which the calculation of base fee can have adverse affects.

get_base_fee() calls the get_Price() function in order to calculate the base fee.


        pub fn get_base_fee(&self) -> Result<Balance, MostError> {
            let gas_price = if let Some(gas_price_oracle_address) = self.data()?.gas_price_oracle {
                let gas_price_oracle: contract_ref!(EthGasPriceOracle) =
                    gas_price_oracle_address.into();

                match gas_price_oracle
                    .call()
@>                  .get_price()
                    .gas_limit(ORACLE_CALL_GAS_LIMIT)
                    .try_invoke()
                {

       . . . some code

        }

and get_price() returns the ETH gas price in Azero which is last updated by owner of gas price oracle.


        fn get_price(&self) -> (u128, u64) {
            (self.last_price, self.last_update)
        }

It can be understood here, if the Most contract is not paused while setting gas price oracle then there could be fetch of incorrect price between old gas price oracle and new gas price oracle. The funtion calculating base fee like send_request() may revert as due to non-functioning of get_price(). send_request() calculates base fee by calling get_base_fee() function.


            let current_base_fee = self.get_base_fee()?;

Therefore, its better to halt the contract when there is update in setting gas price oracle address.

whenever the data is updated the contract is halted. This can be checked in case of set_committee() here

Recommendation to fix\

Ensure contract is halted when set_gas_price_oracle() is called.

        #[ink(message)]
        pub fn set_gas_price_oracle(
            &mut self,
            gas_price_oracle: AccountId,
        ) -> Result<(), MostError> {
            self.ensure_owner()?;
+          self.ensure_halted()?;
            let mut data = self.data()?;
            data.gas_price_oracle = Some(gas_price_oracle);
            self.data.set(&data);
            Ok(())
        }
krzysztofziobro commented 5 months ago

No PoC showing how this can negatively impact the protocol.

0xRizwan commented 4 months ago

@krzysztofziobro

When the data is updated, the following functions are only accessed when the contract is paused.

1) add_pair() 2) emove_pair() 3) set_committee()

Similar to as explained in description above, fn set_gas_price_oracle() should only be called when the contract is paused. Please let me know if this change would affect protocol intended design.