diff --git a/core/src/cluster_info.rs b/core/src/cluster_info.rs index f8a92e1547..96da73a12b 100644 --- a/core/src/cluster_info.rs +++ b/core/src/cluster_info.rs @@ -161,7 +161,7 @@ pub struct PruneData { impl Sanitize for PruneData { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } Ok(()) } @@ -2822,7 +2822,7 @@ mod tests { let mut pd = PruneData::default(); pd.wallclock = MAX_WALLCLOCK; let msg = Protocol::PruneMessage(Pubkey::default(), pd); - assert_eq!(msg.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(msg.sanitize(), Err(SanitizeError::ValueOutOfBounds)); } // computes the maximum size for pull request blooms diff --git a/core/src/contact_info.rs b/core/src/contact_info.rs index d77ba52d80..93fe68333a 100644 --- a/core/src/contact_info.rs +++ b/core/src/contact_info.rs @@ -42,7 +42,7 @@ pub struct ContactInfo { impl Sanitize for ContactInfo { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } Ok(()) } @@ -325,4 +325,12 @@ mod tests { ci.rpc = socketaddr!("127.0.0.1:234"); assert!(ci.valid_client_facing_addr().is_some()); } + + #[test] + fn test_sanitize() { + let mut ci = ContactInfo::default(); + assert_eq!(ci.sanitize(), Ok(())); + ci.wallclock = MAX_WALLCLOCK; + assert_eq!(ci.sanitize(), Err(SanitizeError::ValueOutOfBounds)); + } } diff --git a/core/src/crds_value.rs b/core/src/crds_value.rs index baabbe6334..5265c1c99a 100644 --- a/core/src/crds_value.rs +++ b/core/src/crds_value.rs @@ -83,13 +83,13 @@ impl Sanitize for CrdsData { CrdsData::ContactInfo(val) => val.sanitize(), CrdsData::Vote(ix, val) => { if *ix >= MAX_VOTES { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } val.sanitize() } CrdsData::LowestSlot(ix, val) => { if *ix as usize >= 1 { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } val.sanitize() } @@ -97,7 +97,7 @@ impl Sanitize for CrdsData { CrdsData::AccountsHashes(val) => val.sanitize(), CrdsData::EpochSlots(ix, val) => { if *ix as usize >= MAX_EPOCH_SLOTS as usize { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } val.sanitize() } @@ -115,11 +115,11 @@ pub struct SnapshotHash { impl Sanitize for SnapshotHash { fn sanitize(&self) -> Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } for (slot, _) in &self.hashes { if *slot >= MAX_SLOT { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } } self.from.sanitize() @@ -161,10 +161,10 @@ impl LowestSlot { impl Sanitize for LowestSlot { fn sanitize(&self) -> Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } if self.lowest >= MAX_SLOT { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } if self.root != 0 { return Err(SanitizeError::InvalidValue); @@ -189,7 +189,7 @@ pub struct Vote { impl Sanitize for Vote { fn sanitize(&self) -> Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::Failed); + return Err(SanitizeError::ValueOutOfBounds); } self.from.sanitize()?; self.transaction.sanitize() @@ -448,7 +448,7 @@ mod test { let o = ls.clone(); let v = CrdsValue::new_unsigned(CrdsData::LowestSlot(1, o.clone())); - assert_eq!(v.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(v.sanitize(), Err(SanitizeError::ValueOutOfBounds)); let mut o = ls.clone(); o.slots.insert(1); @@ -505,7 +505,7 @@ mod test { ), &keypair, ); - assert!(item.sanitize().is_err()); + assert_eq!(item.sanitize(), Err(SanitizeError::ValueOutOfBounds)); } #[test] fn test_compute_vote_index_empty() { diff --git a/core/src/epoch_slots.rs b/core/src/epoch_slots.rs index 159dee46ac..b81e5a397c 100644 --- a/core/src/epoch_slots.rs +++ b/core/src/epoch_slots.rs @@ -19,10 +19,10 @@ pub struct Uncompressed { impl Sanitize for Uncompressed { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { if self.first_slot >= MAX_SLOT { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } if self.num >= MAX_SLOTS_PER_ENTRY { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } Ok(()) } @@ -38,10 +38,10 @@ pub struct Flate2 { impl Sanitize for Flate2 { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { if self.first_slot >= MAX_SLOT { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } if self.num >= MAX_SLOTS_PER_ENTRY { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } Ok(()) } @@ -221,7 +221,7 @@ pub struct EpochSlots { impl Sanitize for EpochSlots { fn sanitize(&self) -> std::result::Result<(), SanitizeError> { if self.wallclock >= MAX_WALLCLOCK { - return Err(SanitizeError::ValueOutOfRange); + return Err(SanitizeError::ValueOutOfBounds); } self.from.sanitize()?; self.slots.sanitize() @@ -387,22 +387,22 @@ mod tests { let mut o = slots.clone(); o.first_slot = MAX_SLOT; - assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds)); let mut o = slots.clone(); o.num = MAX_SLOTS_PER_ENTRY; - assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds)); let compressed = Flate2::deflate(slots).unwrap(); assert!(compressed.sanitize().is_ok()); let mut o = compressed.clone(); o.first_slot = MAX_SLOT; - assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds)); let mut o = compressed.clone(); o.num = MAX_SLOTS_PER_ENTRY; - assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds)); let mut slots = EpochSlots::default(); let range: Vec = (0..5000).into_iter().collect(); @@ -412,7 +412,7 @@ mod tests { let mut o = slots.clone(); o.wallclock = MAX_WALLCLOCK; - assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfRange)); + assert_eq!(o.sanitize(), Err(SanitizeError::ValueOutOfBounds)); } #[test] diff --git a/sdk/src/hash.rs b/sdk/src/hash.rs index 45f6f333e8..c255d61768 100644 --- a/sdk/src/hash.rs +++ b/sdk/src/hash.rs @@ -1,5 +1,6 @@ //! The `hash` module provides functions for creating SHA-256 hashes. +use crate::sanitize::Sanitize; use sha2::{Digest, Sha256}; use std::{convert::TryFrom, fmt, mem, str::FromStr}; use thiserror::Error; @@ -30,6 +31,8 @@ impl Hasher { } } +impl Sanitize for Hash {} + impl AsRef<[u8]> for Hash { fn as_ref(&self) -> &[u8] { &self.0[..] diff --git a/sdk/src/instruction.rs b/sdk/src/instruction.rs index ab7cc077ec..16db18a6e9 100644 --- a/sdk/src/instruction.rs +++ b/sdk/src/instruction.rs @@ -1,5 +1,6 @@ //! Defines a composable Instruction type and a memory-efficient CompiledInstruction. +use crate::sanitize::Sanitize; use crate::{pubkey::Pubkey, short_vec, system_instruction::SystemError}; use bincode::serialize; use serde::Serialize; @@ -252,6 +253,8 @@ pub struct CompiledInstruction { pub data: Vec, } +impl Sanitize for CompiledInstruction {} + impl CompiledInstruction { pub fn new(program_ids_index: u8, data: &T, accounts: Vec) -> Self { let data = serialize(data).unwrap(); diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 11d45aa072..4bf59fd0ed 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -184,6 +184,9 @@ impl Sanitize for Message { } } } + self.account_keys.sanitize()?; + self.recent_blockhash.sanitize()?; + self.instructions.sanitize()?; Ok(()) } } diff --git a/sdk/src/sanitize.rs b/sdk/src/sanitize.rs index 6ccc638a56..a564dc9f8f 100644 --- a/sdk/src/sanitize.rs +++ b/sdk/src/sanitize.rs @@ -1,11 +1,22 @@ -#[derive(PartialEq, Debug)] +use thiserror::Error; + +#[derive(PartialEq, Debug, Error, Eq, Clone)] pub enum SanitizeError { - Failed, + #[error("index out of bounds")] IndexOutOfBounds, - ValueOutOfRange, + #[error("value out of bounds")] + ValueOutOfBounds, + #[error("invalid value")] InvalidValue, } +/// Trait for sanitizing values and members of over the wire messages. +/// Implementation should recursively decent through the data structure +/// and sanitize all struct members and enum clauses. Sanitize excludes +/// signature verification checks, those are handled by another pass. +/// Sanitize checks should include but are not limited too: +/// * All index values are in range +/// * All values are within their static max/min bounds pub trait Sanitize { fn sanitize(&self) -> Result<(), SanitizeError> { Ok(())