diff --git a/common/src/types/workloads/work_report.rs b/common/src/types/workloads/work_report.rs index a4a24851..57efa7b4 100644 --- a/common/src/types/workloads/work_report.rs +++ b/common/src/types/workloads/work_report.rs @@ -491,7 +491,10 @@ impl WorkReport { } pub fn total_accumulation_gas_allotted(&self) -> UnsignedGas { - self.digests.iter().map(|wd| wd.accumulate_gas_limit).sum() + self.digests + .iter() + .try_fold(0u64, |acc, wd| acc.checked_add(wd.accumulate_gas_limit)) + .unwrap_or(UnsignedGas::MAX) } pub fn extract_exports_manifest(&self) -> ReportedWorkPackage { diff --git a/pvm/pvm-core/src/state/memory.rs b/pvm/pvm-core/src/state/memory.rs index 19822234..bd317104 100644 --- a/pvm/pvm-core/src/state/memory.rs +++ b/pvm/pvm-core/src/state/memory.rs @@ -159,7 +159,17 @@ impl Memory { if length == 0 { return true; } - let end = start as usize + length - 1; + let start_usize = start as usize; + let end = match start_usize + .checked_add(length) + .and_then(|end| end.checked_sub(1)) + { + Some(end) => end, + None => return false, + }; + if end >= self.data.len() { + return false; + } let (start_page, _) = self.get_page_and_offset(start); let (end_page, _) = self.get_page_and_offset(end as MemAddress); self.is_page_range_readable(start_page..end_page + 1) @@ -205,7 +215,17 @@ impl Memory { if length == 0 { return true; } - let end = start as usize + length - 1; + let start_usize = start as usize; + let end = match start_usize + .checked_add(length) + .and_then(|end| end.checked_sub(1)) + { + Some(end) => end, + None => return false, + }; + if end >= self.data.len() { + return false; + } let (start_page, _) = self.get_page_and_offset(start); let (end_page, _) = self.get_page_and_offset(end as MemAddress); self.is_page_range_writable(start_page..end_page + 1) diff --git a/pvm/pvm-host/src/host_functions/accumulate/mod.rs b/pvm/pvm-host/src/host_functions/accumulate/mod.rs index 2723e746..3f31f72d 100644 --- a/pvm/pvm-host/src/host_functions/accumulate/mod.rs +++ b/pvm/pvm-host/src/host_functions/accumulate/mod.rs @@ -78,15 +78,18 @@ impl AccumulateHostFunction { } // Read always-accumulate services from the memory + let Some(always_accumulate_len) = always_accumulates_count.checked_mul(12) else { + host_call_panic!() + }; if !vm .memory - .is_address_range_readable(always_accumulate_offset, 12 * always_accumulates_count) + .is_address_range_readable(always_accumulate_offset, always_accumulate_len) { host_call_panic!() } let Ok(always_accumulate_services_data) = vm .memory - .read_bytes(always_accumulate_offset, 12 * always_accumulates_count) + .read_bytes(always_accumulate_offset, always_accumulate_len) else { host_call_panic!() }; @@ -552,7 +555,10 @@ impl AccumulateHostFunction { // --- Check Sender Balance (Err: CASH) let amount = vm.read_reg(8); - if accumulator_balance.saturating_sub(amount) < accumulator_threshold_balance { + let Some(remaining_balance) = accumulator_balance.checked_sub(amount) else { + continue_cash!() + }; + if remaining_balance < accumulator_threshold_balance { continue_cash!() } @@ -1198,7 +1204,10 @@ impl AccumulateHostFunction { let service_id = if service_id_reg == u64::MAX { service_id } else { - service_id_reg as ServiceId + let Ok(service_id) = vm.read_reg_as_service_id(7) else { + continue_who!() + }; + service_id }; // Service account not found diff --git a/pvm/pvm-host/src/host_functions/accumulate/tests.rs b/pvm/pvm-host/src/host_functions/accumulate/tests.rs index 8996ca7d..0b55cf60 100644 --- a/pvm/pvm-host/src/host_functions/accumulate/tests.rs +++ b/pvm/pvm-host/src/host_functions/accumulate/tests.rs @@ -4233,6 +4233,84 @@ mod provide_tests { Ok(()) } + #[tokio::test] + async fn test_provide_service_id_out_of_range_returns_who() -> Result<(), Box> { + let fixture = ProvideTestFixture::default(); + let out_of_range_service = (1u64 << 32) + fixture.provide_service as u64; + let vm = fixture + .prepare_vm_builder()? + .with_reg(7, out_of_range_service) + .with_mem_readable_range(fixture.mem_readable_range.clone())? + .build(); + let state_provider = Arc::new(fixture.prepare_state_provider()); + let mut context = fixture + .prepare_invocation_context(state_provider.clone()) + .await?; + + // Check host-call result + let res = AccumulateHostFunction::::host_provide( + fixture.provide_service, + &vm, + state_provider.clone(), + &mut context, + ) + .await?; + assert_eq!(res, ProvideTestFixture::host_call_result_who()); + + // Check partial state after host-call (should remain unchanged) + let x = context.get_mut_accumulate_x().unwrap(); + assert!(!x.provided_preimages.contains(&( + fixture.provide_service, + Octets::from_vec(fixture.preimage_data.clone()) + ))); + Ok(()) + } + + #[tokio::test] + async fn test_provide_invalid_service_id_returns_who() -> Result<(), Box> { + let fixture = ProvideTestFixture::default(); + let invalid_service_id = u64::from(u32::MAX) + 1; + let spoofed_service_id = 0; + let vm = fixture + .prepare_vm_builder()? + .with_reg(7, invalid_service_id) + .with_mem_readable_range(fixture.mem_readable_range.clone())? + .build(); + let state_provider = Arc::new( + MockStateManager::builder() + .with_empty_account(fixture.accumulate_host) + .with_empty_account(spoofed_service_id) + .with_lookups_entry( + spoofed_service_id, + (fixture.preimage_hash.clone(), fixture.preimage_size), + AccountLookupsEntry { + value: AccountLookupsEntryTimeslots::try_from(vec![]).unwrap(), + }, + ), + ); + let mut context = fixture + .prepare_invocation_context(state_provider.clone()) + .await?; + + // Check host-call result + let res = AccumulateHostFunction::::host_provide( + fixture.accumulate_host, + &vm, + state_provider.clone(), + &mut context, + ) + .await?; + assert_eq!(res, ProvideTestFixture::host_call_result_who()); + + // Check partial state after host-call (should remain unchanged) + let x = context.get_mut_accumulate_x().unwrap(); + assert!(!x.provided_preimages.contains(&( + spoofed_service_id, + Octets::from_vec(fixture.preimage_data.clone()) + ))); + Ok(()) + } + #[tokio::test] async fn test_provide_preimage_not_solicited() -> Result<(), Box> { let fixture = ProvideTestFixture::default(); diff --git a/pvm/pvm-host/src/host_functions/general/mod.rs b/pvm/pvm-host/src/host_functions/general/mod.rs index b8098b6c..dd498748 100644 --- a/pvm/pvm-host/src/host_functions/general/mod.rs +++ b/pvm/pvm-host/src/host_functions/general/mod.rs @@ -263,10 +263,13 @@ impl GeneralHostFunction { // --- Check Preimage (Err: NONE) let service_id_reg = vm.read_reg(7); - let service_id = if service_id_reg == u64::MAX || service_id_reg == service_id as u64 { + let service_id = if service_id_reg == u64::MAX { service_id } else { - service_id_reg as ServiceId + let Ok(service_id_reg) = vm.read_reg_as_service_id(7) else { + continue_none!() + }; + service_id_reg }; let Ok(Some(entry)) = accounts_sandbox .get_account_preimages_entry(state_provider, service_id, &hash) @@ -340,7 +343,10 @@ impl GeneralHostFunction { let service_id = if service_id_reg == u64::MAX { service_id } else { - service_id_reg as ServiceId + let Ok(service_id_reg) = vm.read_reg_as_service_id(7) else { + continue_none!() + }; + service_id_reg }; let Ok(Some(entry)) = accounts_sandbox .get_account_storage_entry(state_provider, service_id, &storage_key) @@ -533,7 +539,10 @@ impl GeneralHostFunction { let service_id = if service_id_reg == u64::MAX { service_id } else { - service_id_reg as ServiceId + let Ok(service_id_reg) = vm.read_reg_as_service_id(7) else { + continue_none!() + }; + service_id_reg }; let Ok(Some(metadata)) = accounts_sandbox .get_account_metadata(state_provider, service_id) diff --git a/pvm/pvm-host/src/host_functions/general/tests.rs b/pvm/pvm-host/src/host_functions/general/tests.rs index 124ac7fd..4a3881e2 100644 --- a/pvm/pvm-host/src/host_functions/general/tests.rs +++ b/pvm/pvm-host/src/host_functions/general/tests.rs @@ -1103,6 +1103,34 @@ mod lookup_tests { Ok(()) } + #[tokio::test] + async fn test_lookup_service_id_out_of_range_returns_none() -> Result<(), Box> { + setup_tracing(); + let fixture = LookupTestFixture::default(); + let out_of_range_service_id = (1u64 << 32) + fixture.other_service_id as u64; + let vm = fixture + .prepare_vm_builder()? + .with_reg(7, out_of_range_service_id) + .with_mem_readable_range(fixture.mem_readable_range.clone())? + .with_mem_writable_range(fixture.mem_writable_range.clone())? + .build(); + + let state_provider = + Arc::new(fixture.prepare_state_provider(Some(fixture.other_service_id))); + let mut context = fixture + .prepare_invocation_context(state_provider.clone()) + .await?; + let res = GeneralHostFunction::::host_lookup( + fixture.accumulate_host, + &vm, + state_provider, + &mut context, + ) + .await?; + assert_eq!(res, LookupTestFixture::host_call_result_none()); + Ok(()) + } + #[tokio::test] async fn test_lookup_account_not_found() -> Result<(), Box> { setup_tracing(); @@ -1402,6 +1430,38 @@ mod read_tests { Ok(()) } + #[tokio::test] + async fn test_read_service_id_out_of_range_returns_none() -> Result<(), Box> { + setup_tracing(); + let fixture = ReadTestFixture::default(); + let out_of_range_service_id = (1u64 << 32) + fixture.other_service_id as u64; + let vm = fixture + .prepare_vm_builder()? + .with_reg(7, out_of_range_service_id) + .with_mem_data( + fixture.storage_key_mem_offset, + fixture.storage_key.as_slice(), + )? + .with_mem_readable_range(fixture.mem_readable_range.clone())? + .with_mem_writable_range(fixture.mem_writable_range.clone())? + .build(); + + let state_provider = + Arc::new(fixture.prepare_state_provider(Some(fixture.other_service_id))); + let mut context = fixture + .prepare_invocation_context(state_provider.clone()) + .await?; + let res = GeneralHostFunction::::host_read( + fixture.accumulate_host, + &vm, + state_provider, + &mut context, + ) + .await?; + assert_eq!(res, ReadTestFixture::host_call_result_none()); + Ok(()) + } + #[tokio::test] async fn test_read_account_not_found() -> Result<(), Box> { setup_tracing(); @@ -2061,6 +2121,30 @@ mod info_tests { Ok(()) } + #[tokio::test] + async fn test_info_service_id_out_of_range_returns_none() -> Result<(), Box> { + let fixture = InfoTestFixture::default(); + let out_of_range_service_id = (1u64 << 32) + fixture.info_service_id as u64; + let vm = fixture + .prepare_vm_builder()? + .with_reg(7, out_of_range_service_id) + .build(); + let state_provider = Arc::new(fixture.prepare_state_provider()); + let mut context = fixture + .prepare_invocation_context(state_provider.clone()) + .await?; + + let res = GeneralHostFunction::::host_info( + fixture.accumulate_host, + &vm, + state_provider, + &mut context, + ) + .await?; + assert_eq!(res, InfoTestFixture::host_call_result_none()); + Ok(()) + } + #[tokio::test] async fn test_info_successful_accumulate_host() -> Result<(), Box> { let fixture = InfoTestFixture::default(); diff --git a/pvm/pvm-host/src/host_functions/refine/mod.rs b/pvm/pvm-host/src/host_functions/refine/mod.rs index 2b049210..d06070b8 100644 --- a/pvm/pvm-host/src/host_functions/refine/mod.rs +++ b/pvm/pvm-host/src/host_functions/refine/mod.rs @@ -64,7 +64,7 @@ impl RefineHostFunction { let service_id_reg = vm.read_reg(7); let service_id = if service_id_reg == u64::MAX - || state_provider.account_exists(refine_service_id).await? + && state_provider.account_exists(refine_service_id).await? { refine_service_id } else { diff --git a/pvm/pvm-invocation/src/accumulate/pipeline.rs b/pvm/pvm-invocation/src/accumulate/pipeline.rs index 3408cc0c..aae786e7 100644 --- a/pvm/pvm-invocation/src/accumulate/pipeline.rs +++ b/pvm/pvm-invocation/src/accumulate/pipeline.rs @@ -40,20 +40,27 @@ pub struct OuterAccumulationResult { #[inline] fn max_processable_reports(reports: &[WorkReport], gas_limit: UnsignedGas) -> usize { let mut max_processable = 0; - let mut gas_counter = 0; + let mut gas_counter: UnsignedGas = 0; for report in reports { - let report_gas_usage: UnsignedGas = report - .digests - .iter() - .map(|wd| wd.accumulate_gas_limit) - .sum(); + let mut report_gas_limit: UnsignedGas = 0; + for digest in report.digests.iter() { + let Some(digests_gas_limit_sum) = + report_gas_limit.checked_add(digest.accumulate_gas_limit) + else { + return max_processable; + }; + report_gas_limit = digests_gas_limit_sum; + } - if gas_counter + report_gas_usage > gas_limit { + let Some(next_gas_counter) = gas_counter.checked_add(report_gas_limit) else { + break; + }; + if next_gas_counter > gas_limit { break; } - gas_counter += report_gas_usage; + gas_counter = next_gas_counter; max_processable += 1 } @@ -111,13 +118,14 @@ pub async fn accumulate_outer( Vec::new() }; + let prev_deferred_transfers = deferred_transfers; let ParallelAccumulationResult { service_gas_pairs, new_deferred_transfers, service_output_pairs: output_pairs, } = accumulate_parallel( state_manager.clone(), - Arc::new(deferred_transfers), + Arc::new(prev_deferred_transfers.clone()), Arc::new(reports_to_process), Arc::new(always_accumulate_services), &mut partial_state_union, @@ -130,8 +138,22 @@ pub async fn accumulate_outer( deferred_transfers = new_deferred_transfers; report_idx += processable_reports_prediction; - let gas_used = service_gas_pairs.iter().map(|pair| pair.gas).sum(); - remaining_gas_limit = remaining_gas_limit.saturating_sub(gas_used); + let gas_used = service_gas_pairs + .iter() + .try_fold(0u64, |acc, pair| acc.checked_add(pair.gas)) + .ok_or(PVMInvokeError::GasOverflow)?; + let deferred_transfer_gas = + prev_deferred_transfers + .iter() + .try_fold(0u64, |acc, transfer| { + acc.checked_add(transfer.gas_limit) + .ok_or(PVMInvokeError::GasOverflow) + })?; + remaining_gas_limit = remaining_gas_limit + .checked_add(deferred_transfer_gas) + .ok_or(PVMInvokeError::GasOverflow)? + .checked_sub(gas_used) + .ok_or(PVMInvokeError::GasOverflow)?; service_gas_pairs_flattened.extend(service_gas_pairs); service_output_pairs_flattened.extend(output_pairs.0); } @@ -327,6 +349,25 @@ async fn add_provided_preimages( let preimages_key = hash::(&octets)?; let lookups_key: LookupsKey = (preimages_key.clone(), octets.len() as u32); + // Ignore preimages for missing or removed accounts, or if the lookup request was dropped. + if !partial_state_union + .accounts_sandbox + .account_exists_active(state_manager.clone(), service_id) + .await? + { + continue; + } + let Some(lookups_entry) = partial_state_union + .accounts_sandbox + .get_account_lookups_entry(state_manager.clone(), service_id, &lookups_key) + .await? + else { + continue; + }; + if lookups_entry.timeslots_length() != 0 { + continue; + } + // Insert an entry to the preimages storage partial_state_union .accounts_sandbox @@ -593,22 +634,24 @@ async fn accumulate_single_service( .cloned() .unwrap_or(0); - let reports_gas_aggregated: UnsignedGas = reports + let reports_gas_aggregated = reports .iter() .flat_map(|wr| wr.digests.iter()) .filter(|wd| wd.service_id == service_id) - .map(|wd| wd.accumulate_gas_limit) - .sum(); - - gas_limit += reports_gas_aggregated; + .try_fold(0u64, |acc, wd| acc.checked_add(wd.accumulate_gas_limit)) + .ok_or(PVMInvokeError::GasOverflow)?; + gas_limit = gas_limit + .checked_add(reports_gas_aggregated) + .ok_or(PVMInvokeError::GasOverflow)?; - let deferred_transfers_gas_aggregated: UnsignedGas = prev_deferred_transfers + let deferred_transfers_gas_aggregated = prev_deferred_transfers .iter() .filter(|&transfer| transfer.to == service_id) - .map(|transfer| transfer.gas_limit) - .sum(); - - gas_limit += deferred_transfers_gas_aggregated; + .try_fold(0u64, |acc, transfer| acc.checked_add(transfer.gas_limit)) + .ok_or(PVMInvokeError::GasOverflow)?; + gas_limit = gas_limit + .checked_add(deferred_transfers_gas_aggregated) + .ok_or(PVMInvokeError::GasOverflow)?; AccumulateInvocation::::accumulate( state_manager, diff --git a/pvm/pvm-invocation/src/error.rs b/pvm/pvm-invocation/src/error.rs index 158286dd..e6ee798b 100644 --- a/pvm/pvm-invocation/src/error.rs +++ b/pvm/pvm-invocation/src/error.rs @@ -25,6 +25,8 @@ pub enum PVMInvokeError { MissingAccumulateHostSandbox(ServiceId), #[error("Number of work digests exceeds maximum")] WorkDigestsOverflow, + #[error("Gas usage or gas limit overflowed")] + GasOverflow, #[error("Zero padded justification data size exceeds SEGMENT_SIZE")] PagedProofJustificationBlobTooLarge, #[error("JamCodecError: {0}")] diff --git a/transition/src/state/services.rs b/transition/src/state/services.rs index 7c2e6ff7..ab90b63d 100644 --- a/transition/src/state/services.rs +++ b/transition/src/state/services.rs @@ -208,7 +208,12 @@ async fn transition_service_account( _ => (), } + let metadata_removed = matches!(sandbox.metadata.status(), SandboxEntryStatus::Removed); + for (k, v) in sandbox.storage.iter() { + if metadata_removed && *v.status() != SandboxEntryStatus::Removed { + continue; + } match v.status() { SandboxEntryStatus::Added => { state_manager @@ -265,6 +270,9 @@ async fn transition_service_account( } for (k, v) in sandbox.preimages.iter() { + if metadata_removed && *v.status() != SandboxEntryStatus::Removed { + continue; + } match v.status() { SandboxEntryStatus::Added => { state_manager @@ -320,6 +328,9 @@ async fn transition_service_account( } for (k, v) in sandbox.lookups.iter() { + if metadata_removed && *v.status() != SandboxEntryStatus::Removed { + continue; + } match v.status() { SandboxEntryStatus::Added => { state_manager