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

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

Wrong consideration of time in `GAS_ORACLE_MAX_AGE` #23

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf752eebd9deed4e95e7e9fbfa39c9419222aa4015e225d2173aedc0e9f74632c Severity: medium

Description: Description\

GAS_ORACLE_MAX_AGE is used as duration for fetching the base fee.

    const GAS_ORACLE_MAX_AGE: u64 = 24 * 60 * 60 * 1000; // 1 day

The issue here is, the GAS_ORACLE_MAX_AGE is calculate in milliseconds an the timestamp of the Aleph zero and EVM is in seconds.

The gas oracle duration is intened to be 1 days i.e 24 hours but the current implementation above calculates it as 24000 hours. This is absolutely incorrect.

GAS_ORACLE_MAX_AGE is used in get_base_fee() which is used to query a gas price oracle and returns the current base_fee charged per cross chain transfer denominated in AZERO.

        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()
                {
                    Ok(Ok((gas_price, timestamp))) => {
                        if timestamp + GAS_ORACLE_MAX_AGE <  self.env().block_timestamp() {
                            self.data()?.default_gas_price
                        } else if gas_price < self.data()?.min_gas_price {
                            self.data()?.min_gas_price
                        } else if gas_price > self.data()?.max_gas_price {
                            self.data()?.max_gas_price
                        } else {
                            gas_price
                        }
                    }
                    _ => self.data()?.default_gas_price,
                }

Now, see this line of code,

                        if timestamp + GAS_ORACLE_MAX_AGE < self.env().block_timestamp() {
                            self.data()?.default_gas_price

This line of code will not be executed till the timestamp + 24000 hours is reached, however it should be 24 hours to pass the condition to get the default gas price. With current implementation, it will take more than 2.5 years to pass this condition in order to get the default gas price.

POC The gas price oracle duration to fetch is price is expected to be 24 hours, However the current implementation sets it to 24000 hours which makes the use of GAS_ORACLE_MAX_AGE in most contract after more than 2.5 years. This is not expected behaviour as the user would get the stale price which would only be updated after 2.5 years.

Recommendation to fix\

-    const GAS_ORACLE_MAX_AGE: u64 = 24 * 60 * 60 * 1000; // 1 day
+    const GAS_ORACLE_MAX_AGE: u64 = 24 * 60 * 60; // 1 day
0xRizwan commented 7 months ago

This should be invalid. Time is passed in milliseconds in ink based projects, so i think current implementation is correct.

Correct me if i am wrong.

krzysztofziobro commented 7 months ago

Yes, ink! has a timestamp measured in milliseconds, so this is not an issue