From 067e29ae0b3b3a7a0af51f7414f7e4262537f0c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Tue, 9 Nov 2021 13:35:49 +0100 Subject: [PATCH] Replaces unchecked integer arithmetic by guarded versions. (#21186) --- program-runtime/src/instruction_processor.rs | 31 ++++++++++++-------- program-runtime/src/invoke_context.rs | 30 ++++++++++++------- program-runtime/src/lib.rs | 1 - program-runtime/src/log_collector.rs | 6 ++-- 4 files changed, 42 insertions(+), 26 deletions(-) diff --git a/program-runtime/src/instruction_processor.rs b/program-runtime/src/instruction_processor.rs index 48c2dbad0..6697e4652 100644 --- a/program-runtime/src/instruction_processor.rs +++ b/program-runtime/src/instruction_processor.rs @@ -58,14 +58,20 @@ pub struct ExecuteDetailsTimings { } impl ExecuteDetailsTimings { pub fn accumulate(&mut self, other: &ExecuteDetailsTimings) { - self.serialize_us += other.serialize_us; - self.create_vm_us += other.create_vm_us; - self.execute_us += other.execute_us; - self.deserialize_us += other.deserialize_us; - self.changed_account_count += other.changed_account_count; - self.total_account_count += other.total_account_count; - self.total_data_size += other.total_data_size; - self.data_size_changed += other.data_size_changed; + self.serialize_us = self.serialize_us.saturating_add(other.serialize_us); + self.create_vm_us = self.create_vm_us.saturating_add(other.create_vm_us); + self.execute_us = self.execute_us.saturating_add(other.execute_us); + self.deserialize_us = self.deserialize_us.saturating_add(other.deserialize_us); + self.changed_account_count = self + .changed_account_count + .saturating_add(other.changed_account_count); + self.total_account_count = self + .total_account_count + .saturating_add(other.total_account_count); + self.total_data_size = self.total_data_size.saturating_add(other.total_data_size); + self.data_size_changed = self + .data_size_changed + .saturating_add(other.data_size_changed); for (id, other) in &other.per_program_timings { let program_timing = self.per_program_timings.entry(*id).or_default(); program_timing.accumulated_us = program_timing @@ -209,8 +215,8 @@ impl PreAccount { } if outermost_call { - timings.total_account_count += 1; - timings.total_data_size += post.data().len(); + timings.total_account_count = timings.total_account_count.saturating_add(1); + timings.total_data_size = timings.total_data_size.saturating_add(post.data().len()); if owner_changed || lamports_changed || data_len_changed @@ -218,8 +224,9 @@ impl PreAccount { || rent_epoch_changed || self.changed { - timings.changed_account_count += 1; - timings.data_size_changed += post.data().len(); + timings.changed_account_count = timings.changed_account_count.saturating_add(1); + timings.data_size_changed = + timings.data_size_changed.saturating_add(post.data().len()); } } diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 357ff1a74..35091483a 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -300,8 +300,9 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { .try_borrow_mut() .map_err(|_| InstructionError::AccountBorrowOutstanding)?; } + let pre_account = &self.pre_accounts[unique_index]; let account = self.accounts[account_index].1.borrow(); - self.pre_accounts[unique_index] + pre_account .verify( program_id, message.is_writable(account_index, demote_program_write_locks), @@ -315,13 +316,17 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { ic_logger_msg!( self.logger, "failed to verify account {}: {}", - self.pre_accounts[unique_index].key(), + pre_account.key(), err ); err })?; - pre_sum += u128::from(self.pre_accounts[unique_index].lamports()); - post_sum += u128::from(account.lamports()); + pre_sum = pre_sum + .checked_add(u128::from(pre_account.lamports())) + .ok_or(InstructionError::UnbalancedInstruction)?; + post_sum = post_sum + .checked_add(u128::from(account.lamports())) + .ok_or(InstructionError::UnbalancedInstruction)?; Ok(()) }; instruction.visit_each_account(&mut work)?; @@ -383,8 +388,12 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { ic_logger_msg!(logger, "failed to verify account {}: {}", key, err); err })?; - pre_sum += u128::from(pre_account.lamports()); - post_sum += u128::from(account.lamports()); + pre_sum = pre_sum + .checked_add(u128::from(pre_account.lamports())) + .ok_or(InstructionError::UnbalancedInstruction)?; + post_sum = post_sum + .checked_add(u128::from(account.lamports())) + .ok_or(InstructionError::UnbalancedInstruction)?; if is_writable && !pre_account.executable() { pre_account.update(&account); } @@ -466,10 +475,10 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { execute_us: u64, deserialize_us: u64, ) { - self.timings.serialize_us += serialize_us; - self.timings.create_vm_us += create_vm_us; - self.timings.execute_us += execute_us; - self.timings.deserialize_us += deserialize_us; + self.timings.serialize_us = self.timings.serialize_us.saturating_add(serialize_us); + self.timings.create_vm_us = self.timings.create_vm_us.saturating_add(create_vm_us); + self.timings.execute_us = self.timings.execute_us.saturating_add(execute_us); + self.timings.deserialize_us = self.timings.deserialize_us.saturating_add(deserialize_us); } fn get_sysvars(&self) -> &[(Pubkey, Vec)] { self.sysvars @@ -664,6 +673,7 @@ mod tests { ModifyReadonly, } + #[allow(clippy::integer_arithmetic)] fn mock_process_instruction( first_instruction_account: usize, data: &[u8], diff --git a/program-runtime/src/lib.rs b/program-runtime/src/lib.rs index b35880ebb..f6dcc7cd3 100644 --- a/program-runtime/src/lib.rs +++ b/program-runtime/src/lib.rs @@ -1,5 +1,4 @@ #![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] -#![allow(clippy::integer_arithmetic)] // TODO: Remove pub mod instruction_processor; pub mod instruction_recorder; diff --git a/program-runtime/src/log_collector.rs b/program-runtime/src/log_collector.rs index 382f36afd..7c5a452a4 100644 --- a/program-runtime/src/log_collector.rs +++ b/program-runtime/src/log_collector.rs @@ -17,14 +17,14 @@ pub struct LogCollector { impl LogCollector { pub fn log(&self, message: &str) { let mut inner = self.inner.borrow_mut(); - - if inner.bytes_written + message.len() >= LOG_MESSAGES_BYTES_LIMIT { + let bytes_written = inner.bytes_written.saturating_add(message.len()); + if bytes_written >= LOG_MESSAGES_BYTES_LIMIT { if !inner.limit_warning { inner.limit_warning = true; inner.messages.push(String::from("Log truncated")); } } else { - inner.bytes_written += message.len(); + inner.bytes_written = bytes_written; inner.messages.push(message.to_string()); } }