Unbounded Operation in `local_cache::apply_quotas` Can Cause OOG

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/chain-extension/src/local_cache.rs#L223 https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/chain-extension/src/local_cache.rs#L222-L223

Vulnerability details

Impact

Problem occurs due to lack of limiting the number of iterations in the apply_quotas function. This function iterates over a collection of quotas and applies them to storage. Without constraining the loop iterations, the function can consume excessive gas, potentially leading to out-of-gas exceptions and Denial of Service (DoS) attacks.

Proof of Concept

by providing a large number of quotas to the apply_quotas function, causing it to iterate excessively and consume all available gas in a block. Consequently, subsequent transactions may fail due to insufficient gas, resulting in a successful DoS attack.

   pub fn apply_quotas<'a>(&mut self, quotas: impl IntoIterator<Item = (&'a [u8], usize)>) {
        for (contract, max_size) in quotas.into_iter() {
            log::trace!(
                "Applying cache quotas for {} max_size={max_size}",
                hex_fmt::HexFmt(contract)
            );
            if max_size == 0 {
                self.storages.remove(contract);
                continue;
            }
            match self.storages.get_mut(contract) {
                Some(store) => {
                    store.max_size = max_size;
                    store.fit_size();
                }
                None => {
                    self.storages
                        .insert(contract.to_vec(), Storage::new(max_size));
                }
            }
        }
    }
}

Tools Used

Manual

Recommended Mitigation Steps

By Implemeningg a constraint mechanism to limit the number of iterations in the loop within the apply_quotas function. This ensures that the function does not consume excessive gas.

Here the possible workaround for this :

   pub fn apply_quotas<'a>(&mut self, quotas: impl IntoIterator<Item = (&'a [u8], usize)>) {
+           // Define a maximum limit on the number of iterations
+           const MAX_ITERATIONS: usize = 100; // Adjust as needed
+           let mut iterations = 0; // Initialize iteration counter

        for (contract, max_size) in quotas.into_iter() {

+           // Check if maximum iterations exceeded
+           if iterations >= MAX_ITERATIONS {
+           log::warn!("Exceeded maximum iterations in apply_quotas function");
+            break;
+            }

+           // Increment iteration counter
+           iterations += 1;
            log::trace!(
                "Applying cache quotas for {} max_size={max_size}",
                hex_fmt::HexFmt(contract)
            );
            if max_size == 0 {
                self.storages.remove(contract);
                continue;
            }
            match self.storages.get_mut(contract) {
                Some(store) => {
                    store.max_size = max_size;
                    store.fit_size();
                }
                None => {
                    self.storages
                        .insert(contract.to_vec(), Storage::new(max_size));
                }
            }
        }
    }
}

if the current iteration exceeds the maximum limit. If it does, a warning message is logged, and the loop is terminated to prevent excessive gas consumption and potential DoS attacks.

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[141345]

seems invalid, it is not about gas

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Invalid

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Invalid