diff --git a/node/bft/ledger-service/Cargo.toml b/node/bft/ledger-service/Cargo.toml index f5b3f6b22b..da7d606161 100644 --- a/node/bft/ledger-service/Cargo.toml +++ b/node/bft/ledger-service/Cargo.toml @@ -86,3 +86,7 @@ workspace = true workspace = true optional = true features = [ "log" ] + +[dev-dependencies.snarkvm] +workspace = true +features = [ "test-helpers" ] diff --git a/node/bft/ledger-service/src/helpers.rs b/node/bft/ledger-service/src/helpers.rs new file mode 100644 index 0000000000..1f5ca1267e --- /dev/null +++ b/node/bft/ledger-service/src/helpers.rs @@ -0,0 +1,171 @@ +// Copyright (c) 2019-2026 Provable Inc. +// This file is part of the snarkOS library. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at: + +// http://www.apache.org/licenses/LICENSE-2.0 + +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use snarkvm::{ + ledger::{Transaction, narwhal::Data, puzzle::Solution}, + prelude::{FromBytes, Network, Result}, +}; + +use anyhow::ensure; +use std::io::Cursor; + +/// Deserializes a transaction, requiring raw buffers to contain exactly one serialized transaction. +/// +/// For buffered transmissions, this rejects buffers that exceed the network transaction size limit or leave trailing +/// bytes after `Transaction::read_le` succeeds. Object transmissions are returned as-is. +pub fn deserialize_transaction_strict(transaction: Data>) -> Result> { + match transaction { + Data::Object(transaction) => Ok(transaction), + Data::Buffer(bytes) => { + ensure!( + bytes.len() <= N::LATEST_MAX_TRANSACTION_SIZE(), + "Transaction exceeds maximum size - {} bytes > {} bytes", + bytes.len(), + N::LATEST_MAX_TRANSACTION_SIZE() + ); + + let bytes_len = u64::try_from(bytes.len())?; + let mut reader = Cursor::new(bytes.as_ref()); + let transaction = Transaction::::read_le(&mut reader)?; + ensure!( + reader.position() == bytes_len, + "Transaction buffer contains {} trailing bytes", + bytes_len.saturating_sub(reader.position()) + ); + Ok(transaction) + } + } +} + +/// Deserializes a solution, requiring raw buffers to contain exactly one serialized solution. +/// +/// For buffered transmissions, this rejects buffers that leave trailing bytes after `Solution::read_le` succeeds. +/// Object transmissions are returned as-is. +pub fn deserialize_solution_strict(solution: Data>) -> Result> { + match solution { + Data::Object(solution) => Ok(solution), + Data::Buffer(bytes) => { + let bytes_len = u64::try_from(bytes.len())?; + let mut reader = Cursor::new(bytes.as_ref()); + let solution = Solution::::read_le(&mut reader)?; + ensure!( + reader.position() == bytes_len, + "Solution buffer contains {} trailing bytes", + bytes_len.saturating_sub(reader.position()) + ); + Ok(solution) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use snarkvm::{ + console::account::{Address, PrivateKey}, + ledger::{puzzle::PartialSolution, test_helpers::sample_execution_transaction_with_fee}, + prelude::{MainnetV0, Rng, TestRng, ToBytes}, + }; + + type CurrentNetwork = MainnetV0; + + fn sample_solution(rng: &mut TestRng) -> Solution { + let private_key = PrivateKey::::new(rng).unwrap(); + let address = Address::try_from(private_key).unwrap(); + let partial_solution = PartialSolution::new(rng.random(), address, rng.random()).unwrap(); + Solution::new(partial_solution, rng.random()) + } + + #[test] + fn deserialize_transaction_strict_accepts_object() -> Result<()> { + let rng = &mut TestRng::default(); + let expected = sample_execution_transaction_with_fee(false, rng, 0); + + let actual = deserialize_transaction_strict(Data::Object(expected.clone()))?; + + assert_eq!(actual, expected); + Ok(()) + } + + #[test] + fn deserialize_transaction_strict_accepts_canonical_buffer() -> Result<()> { + let rng = &mut TestRng::default(); + let expected = sample_execution_transaction_with_fee(false, rng, 0); + let bytes = expected.to_bytes_le()?; + + let actual = deserialize_transaction_strict(Data::Buffer(bytes.into()))?; + + assert_eq!(actual, expected); + Ok(()) + } + + #[test] + fn deserialize_transaction_strict_rejects_padded_buffer() -> Result<()> { + let rng = &mut TestRng::default(); + let transaction = sample_execution_transaction_with_fee(false, rng, 0); + let mut bytes = transaction.to_bytes_le()?; + bytes.push(0); + + let result = deserialize_transaction_strict::(Data::Buffer(bytes.into())); + + assert!(result.unwrap_err().to_string().contains("trailing bytes")); + Ok(()) + } + + #[test] + fn deserialize_transaction_strict_rejects_oversize_buffer() { + let bytes = vec![0; CurrentNetwork::LATEST_MAX_TRANSACTION_SIZE() + 1]; + + let result = deserialize_transaction_strict::(Data::Buffer(bytes.into())); + + assert!(result.unwrap_err().to_string().contains("exceeds maximum size")); + } + + #[test] + fn deserialize_solution_strict_accepts_object() -> Result<()> { + let rng = &mut TestRng::default(); + let expected = sample_solution(rng); + + let actual = deserialize_solution_strict(Data::Object(expected))?; + + assert_eq!(actual, expected); + Ok(()) + } + + #[test] + fn deserialize_solution_strict_accepts_exact_buffer() -> Result<()> { + let rng = &mut TestRng::default(); + let expected = sample_solution(rng); + let bytes = expected.to_bytes_le()?; + + let actual = deserialize_solution_strict(Data::Buffer(bytes.into()))?; + + assert_eq!(actual, expected); + Ok(()) + } + + #[test] + fn deserialize_solution_strict_rejects_padded_buffer() -> Result<()> { + let rng = &mut TestRng::default(); + let solution = sample_solution(rng); + let mut bytes = solution.to_bytes_le()?; + bytes.push(0); + + let result = deserialize_solution_strict::(Data::Buffer(bytes.into())); + + assert!(result.unwrap_err().to_string().contains("trailing bytes")); + Ok(()) + } +} diff --git a/node/bft/ledger-service/src/ledger.rs b/node/bft/ledger-service/src/ledger.rs index 7d3d3ec136..483ee87d91 100644 --- a/node/bft/ledger-service/src/ledger.rs +++ b/node/bft/ledger-service/src/ledger.rs @@ -13,7 +13,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{BeginLedgerUpdateError, LedgerService, LedgerUpdateService, fmt_id, spawn_blocking}; +use crate::{ + BeginLedgerUpdateError, + LedgerService, + LedgerUpdateService, + deserialize_solution_strict, + deserialize_transaction_strict, + fmt_id, + spawn_blocking, +}; use snarkos_utilities::Stoppable; #[cfg(feature = "test_network")] @@ -37,7 +45,6 @@ use snarkvm::{ Address, ConsensusVersion, Field, - FromBytes, Network, Result, bail, @@ -63,7 +70,7 @@ use parking_lot::{Mutex, RwLock}; #[cfg(not(feature = "serial"))] use rayon::prelude::*; -use std::{fmt, io::Read, ops::Range, sync::Arc}; +use std::{fmt, ops::Range, sync::Arc}; /// A core ledger service. #[allow(clippy::type_complexity)] @@ -388,12 +395,7 @@ impl> LedgerService for CoreLedgerService< Transmission::Transaction(transaction_data), ) => { // Deserialize the transaction. If the transaction exceeds the maximum size, then return an error. - let transaction = match transaction_data.clone() { - Data::Object(transaction) => transaction, - Data::Buffer(bytes) => { - Transaction::::read_le(&mut bytes.take(N::LATEST_MAX_TRANSACTION_SIZE() as u64))? - } - }; + let transaction = deserialize_transaction_strict(transaction_data.clone())?; // Ensure the transaction ID matches the expected transaction ID. if transaction.id() != expected_transaction_id { bail!( @@ -424,7 +426,7 @@ impl> LedgerService for CoreLedgerService< TransmissionID::Solution(expected_solution_id, expected_checksum), Transmission::Solution(solution_data), ) => { - match solution_data.clone().deserialize_blocking() { + match deserialize_solution_strict(solution_data.clone()) { Ok(solution) => { if solution.id() != expected_solution_id { bail!( @@ -462,7 +464,7 @@ impl> LedgerService for CoreLedgerService< /// Checks the given solution is well-formed. async fn check_solution_basic(&self, solution_id: SolutionID, solution: Data>) -> Result<()> { // Deserialize the solution. - let solution = spawn_blocking!(solution.deserialize_blocking())?; + let solution = spawn_blocking!(deserialize_solution_strict(solution))?; // Ensure the solution ID matches in the solution. if solution_id != solution.id() { bail!("Invalid solution - expected {solution_id}, found {}", solution.id()); diff --git a/node/bft/ledger-service/src/lib.rs b/node/bft/ledger-service/src/lib.rs index be7c1aba52..3d5da7790c 100644 --- a/node/bft/ledger-service/src/lib.rs +++ b/node/bft/ledger-service/src/lib.rs @@ -22,6 +22,9 @@ extern crate async_trait; #[cfg(feature = "metrics")] extern crate snarkos_node_metrics as metrics; +mod helpers; +pub use helpers::*; + #[cfg(feature = "ledger")] pub mod ledger; #[cfg(feature = "ledger")] diff --git a/node/bft/src/primary.rs b/node/bft/src/primary.rs index c229ce4a08..d7dc9ba81c 100644 --- a/node/bft/src/primary.rs +++ b/node/bft/src/primary.rs @@ -48,7 +48,7 @@ use crate::{ use snarkos_account::Account; use snarkos_node_bft_events::PrimaryPing; -use snarkos_node_bft_ledger_service::LedgerService; +use snarkos_node_bft_ledger_service::{LedgerService, deserialize_transaction_strict}; #[cfg(test)] use snarkos_node_network::ConnectionMode; use snarkos_node_network::PeerPoolHandling; @@ -708,14 +708,7 @@ impl proposal_task::BatchPropose for Primary { } // Deserialize the transaction. If the transaction exceeds the maximum size, then return an error. - let transaction = spawn_blocking!({ - match transaction { - Data::Object(transaction) => Ok(transaction), - Data::Buffer(bytes) => Ok(Transaction::::read_le( - &mut bytes.take(N::LATEST_MAX_TRANSACTION_SIZE() as u64), - )?), - } - })?; + let transaction = spawn_blocking!(deserialize_transaction_strict(transaction))?; // Fetch the current block height and consensus version. let current_block_height = self.ledger.latest_block_height(); diff --git a/node/bft/src/worker.rs b/node/bft/src/worker.rs index eec942f965..4a7a5523f3 100644 --- a/node/bft/src/worker.rs +++ b/node/bft/src/worker.rs @@ -25,7 +25,7 @@ use crate::{ helpers::{Pending, Ready, Storage, WorkerReceiver, fmt_id, max_redundant_requests}, spawn_blocking, }; -use snarkos_node_bft_ledger_service::LedgerService; +use snarkos_node_bft_ledger_service::{LedgerService, deserialize_transaction_strict}; use snarkvm::{ console::prelude::*, ledger::{ @@ -417,14 +417,7 @@ impl Worker { return Ok(false); } // Deserialize the transaction. If the transaction exceeds the maximum size, then return an error. - let transaction = spawn_blocking!({ - match transaction { - Data::Object(transaction) => Ok(transaction), - Data::Buffer(bytes) => { - Ok(Transaction::::read_le(&mut bytes.take(N::LATEST_MAX_TRANSACTION_SIZE() as u64))?) - } - } - })?; + let transaction = spawn_blocking!(deserialize_transaction_strict(transaction))?; // Check that the transaction is well-formed and unique. self.ledger.check_transaction_basic(transaction_id, transaction).await?;