diff --git a/message/src/compiled_instruction.rs b/message/src/compiled_instruction.rs index c8c05002f..6022662fc 100644 --- a/message/src/compiled_instruction.rs +++ b/message/src/compiled_instruction.rs @@ -4,7 +4,10 @@ use serde_derive::{Deserialize, Serialize}; use solana_frozen_abi_macro::AbiExample; #[cfg(feature = "wincode")] use wincode::{containers, len::ShortU16Len, SchemaRead, SchemaWrite}; -use {solana_address::Address, solana_sanitize::Sanitize}; +use { + solana_address::Address, + solana_sanitize::{Sanitize, SanitizeError}, +}; /// A compact encoding of an instruction. /// @@ -34,7 +37,14 @@ pub struct CompiledInstruction { pub data: Vec, } -impl Sanitize for CompiledInstruction {} +impl Sanitize for CompiledInstruction { + fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> Result<(), SanitizeError> { + if limit_ix_accounts_simd_406 && self.accounts.len() > 255 { + return Err(SanitizeError::ValueOutOfBounds); + } + Ok(()) + } +} impl CompiledInstruction { #[cfg(feature = "bincode")] diff --git a/message/src/legacy.rs b/message/src/legacy.rs index 3c2c191b0..244210e07 100644 --- a/message/src/legacy.rs +++ b/message/src/legacy.rs @@ -103,7 +103,7 @@ pub struct Message { } impl Sanitize for Message { - fn sanitize(&self) -> std::result::Result<(), SanitizeError> { + fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> std::result::Result<(), SanitizeError> { // signing area and read-only non-signing area should not overlap if self.header.num_required_signatures as usize + self.header.num_readonly_unsigned_accounts as usize @@ -131,9 +131,9 @@ impl Sanitize for Message { } } } - self.account_keys.sanitize()?; - self.recent_blockhash.sanitize()?; - self.instructions.sanitize()?; + self.account_keys.sanitize(limit_ix_accounts_simd_406)?; + self.recent_blockhash.sanitize(limit_ix_accounts_simd_406)?; + self.instructions.sanitize(limit_ix_accounts_simd_406)?; Ok(()) } } @@ -893,4 +893,27 @@ mod tests { assert!(!message5.is_writable_index(0)); assert!(!message5.is_writable_index(1)); } + + #[test] + fn test_more_than_255_accounts_in_ix() { + let mut accounts: Vec = (0..255).collect(); + accounts.push(5); + accounts.push(4); + let message = crate::v0::Message { + account_keys: vec![Address::new_unique(); 256], + instructions: vec![CompiledInstruction::new_from_raw_parts( + 4, + Vec::new(), + accounts, + )], + header: MessageHeader { + num_required_signatures: 2, + num_readonly_signed_accounts: 1, + num_readonly_unsigned_accounts: 0, + }, + ..crate::v0::Message::default() + }; + let result = message.sanitize(true); + assert_eq!(result.err().unwrap(), SanitizeError::ValueOutOfBounds); + } } diff --git a/message/src/sanitized.rs b/message/src/sanitized.rs index 279d5e057..a2baf7715 100644 --- a/message/src/sanitized.rs +++ b/message/src/sanitized.rs @@ -110,8 +110,9 @@ impl SanitizedMessage { pub fn try_from_legacy_message( message: legacy::Message, reserved_account_keys: &HashSet
, + limit_ix_accounts_simd_406: bool, ) -> Result { - message.sanitize()?; + message.sanitize(limit_ix_accounts_simd_406)?; Ok(Self::Legacy(LegacyMessage::new( message, reserved_account_keys, @@ -456,6 +457,7 @@ mod tests { SanitizedMessage::try_from_legacy_message( legacy_message_with_no_signers, &HashSet::default(), + true, ) .err(), Some(SanitizeMessageError::IndexOutOfBounds), @@ -482,6 +484,7 @@ mod tests { ..legacy::Message::default() }, &HashSet::default(), + true, ) .unwrap(); @@ -529,6 +532,7 @@ mod tests { instructions, ), &HashSet::default(), + true, ) .unwrap(); @@ -571,6 +575,7 @@ mod tests { ..legacy::Message::default() }, &HashSet::default(), + true, ) .unwrap(); match legacy_message { @@ -655,6 +660,7 @@ mod tests { ], ), &HashSet::new(), + true, ) .unwrap(); @@ -688,6 +694,7 @@ mod tests { ..legacy::Message::default() }, &HashSet::default(), + true, ) .unwrap(); assert_eq!(legacy_message.static_account_keys(), &keys); diff --git a/message/src/versions/mod.rs b/message/src/versions/mod.rs index df16f4f35..c91438c93 100644 --- a/message/src/versions/mod.rs +++ b/message/src/versions/mod.rs @@ -59,10 +59,10 @@ pub enum VersionedMessage { } impl VersionedMessage { - pub fn sanitize(&self) -> Result<(), SanitizeError> { + pub fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> Result<(), SanitizeError> { match self { - Self::Legacy(message) => message.sanitize(), - Self::V0(message) => message.sanitize(), + Self::Legacy(message) => message.sanitize(limit_ix_accounts_simd_406), + Self::V0(message) => message.sanitize(limit_ix_accounts_simd_406), } } diff --git a/message/src/versions/sanitized.rs b/message/src/versions/sanitized.rs index 6e836e89b..cbfad02b0 100644 --- a/message/src/versions/sanitized.rs +++ b/message/src/versions/sanitized.rs @@ -12,13 +12,16 @@ pub struct SanitizedVersionedMessage { impl TryFrom for SanitizedVersionedMessage { type Error = SanitizeError; fn try_from(message: VersionedMessage) -> Result { - Self::try_new(message) + Self::try_new(message, true) } } impl SanitizedVersionedMessage { - pub fn try_new(message: VersionedMessage) -> Result { - message.sanitize()?; + pub fn try_new( + message: VersionedMessage, + limit_ix_accounts_simd_406: bool, + ) -> Result { + message.sanitize(limit_ix_accounts_simd_406)?; Ok(Self { message }) } diff --git a/message/src/versions/v0/mod.rs b/message/src/versions/v0/mod.rs index 13b4171dd..1758362eb 100644 --- a/message/src/versions/v0/mod.rs +++ b/message/src/versions/v0/mod.rs @@ -110,7 +110,7 @@ pub struct Message { impl Message { /// Sanitize message fields and compiled instruction indexes - pub fn sanitize(&self) -> Result<(), SanitizeError> { + pub fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> Result<(), SanitizeError> { let num_static_account_keys = self.account_keys.len(); if usize::from(self.header.num_required_signatures) .saturating_add(usize::from(self.header.num_readonly_unsigned_accounts)) @@ -187,6 +187,9 @@ impl Message { return Err(SanitizeError::IndexOutOfBounds); } } + if limit_ix_accounts_simd_406 && ci.accounts.len() > 255 { + return Err(SanitizeError::ValueOutOfBounds); + } } Ok(()) @@ -407,7 +410,7 @@ mod tests { account_keys: vec![Address::new_unique()], ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -426,7 +429,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -445,7 +448,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -470,7 +473,7 @@ mod tests { ..Message::default() }; - assert!(message.sanitize().is_err()); + assert!(message.sanitize(true).is_err()); } #[test] @@ -493,7 +496,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -504,7 +507,7 @@ mod tests { account_keys: vec![Address::new_unique()], ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -519,7 +522,7 @@ mod tests { account_keys: vec![Address::new_unique()], ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -538,7 +541,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -552,7 +555,7 @@ mod tests { account_keys: (0..=u8::MAX).map(|_| Address::new_unique()).collect(), ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -566,7 +569,7 @@ mod tests { account_keys: (0..=256).map(|_| Address::new_unique()).collect(), ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -585,7 +588,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_ok()); } @@ -604,7 +607,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -629,7 +632,7 @@ mod tests { ..Message::default() }; - assert!(message.sanitize().is_err()); + assert!(message.sanitize(true).is_err()); } #[test] @@ -652,7 +655,7 @@ mod tests { }], ..Message::default() } - .sanitize() + .sanitize(true) .is_err()); } @@ -788,4 +791,27 @@ mod tests { assert!(!message.is_account_maybe_reserved(3, None)); assert!(!message.is_account_maybe_reserved(4, None)); } + + #[test] + fn test_more_than_255_accounts_in_ix() { + let mut accounts: Vec = (0..255).collect(); + accounts.push(5); + accounts.push(4); + let message = Message { + account_keys: vec![Address::new_unique(); 256], + instructions: vec![CompiledInstruction::new_from_raw_parts( + 4, + Vec::new(), + accounts, + )], + header: MessageHeader { + num_required_signatures: 2, + num_readonly_signed_accounts: 1, + num_readonly_unsigned_accounts: 0, + }, + ..Message::default() + }; + let result = message.sanitize(true); + assert_eq!(result.err().unwrap(), SanitizeError::ValueOutOfBounds); + } } diff --git a/sanitize/src/lib.rs b/sanitize/src/lib.rs index 9dc73f8a6..03cd83576 100644 --- a/sanitize/src/lib.rs +++ b/sanitize/src/lib.rs @@ -34,15 +34,15 @@ impl fmt::Display for SanitizeError { /// - All index values are in range. /// - All values are within their static max/min bounds. pub trait Sanitize { - fn sanitize(&self) -> Result<(), SanitizeError> { + fn sanitize(&self, _limit_ix_accounts_simd_406: bool) -> Result<(), SanitizeError> { Ok(()) } } impl Sanitize for [T] { - fn sanitize(&self) -> Result<(), SanitizeError> { + fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> Result<(), SanitizeError> { for x in self.iter() { - x.sanitize()?; + x.sanitize(limit_ix_accounts_simd_406)?; } Ok(()) } diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index d4d47e67b..9bce9c699 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -33,6 +33,7 @@ fn bench_construct_instructions_data(b: &mut Bencher) { let message = SanitizedMessage::try_from_legacy_message( Message::new(&instructions, Some(&Pubkey::new_unique())), &HashSet::default(), + true, ) .unwrap(); b.iter(|| { @@ -56,6 +57,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { let message = SanitizedMessage::try_from_legacy_message( Message::new(&instructions, Some(&Pubkey::new_unique())), &HashSet::default(), + true, ) .unwrap(); let serialized = construct_instructions_data(&message.decompile_instructions()); @@ -73,6 +75,7 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { let message = SanitizedMessage::try_from_legacy_message( Message::new(&instructions, Some(&Pubkey::new_unique())), &HashSet::default(), + true, ) .unwrap(); let serialized = construct_instructions_data(&message.decompile_instructions()); diff --git a/transaction/src/lib.rs b/transaction/src/lib.rs index c27f28680..8558abc71 100644 --- a/transaction/src/lib.rs +++ b/transaction/src/lib.rs @@ -199,14 +199,14 @@ pub struct Transaction { } impl Sanitize for Transaction { - fn sanitize(&self) -> result::Result<(), SanitizeError> { + fn sanitize(&self, limit_ix_accounts_simd_406: bool) -> result::Result<(), SanitizeError> { if self.message.header.num_required_signatures as usize > self.signatures.len() { return Err(SanitizeError::IndexOutOfBounds); } if self.signatures.len() > self.message.account_keys.len() { return Err(SanitizeError::IndexOutOfBounds); } - self.message.sanitize() + self.message.sanitize(limit_ix_accounts_simd_406) } } @@ -1141,7 +1141,7 @@ mod tests { vec![prog1, prog2], instructions, ); - assert!(tx.sanitize().is_ok()); + assert!(tx.sanitize(true).is_ok()); assert_eq!(tx.key(0, 0), Some(&key.pubkey())); assert_eq!(tx.signer_key(0, 0), Some(&key.pubkey())); @@ -1176,7 +1176,7 @@ mod tests { vec![], instructions, ); - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); } #[test] fn test_refs_invalid_account() { @@ -1190,7 +1190,7 @@ mod tests { instructions, ); assert_eq!(*get_program_id(&tx, 0), Address::default()); - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); } #[test] @@ -1208,50 +1208,50 @@ mod tests { ); let mut tx = Transaction::new_with_payer(&[ix], Some(&key.pubkey())); let o = tx.clone(); - assert_eq!(tx.sanitize(), Ok(())); + assert_eq!(tx.sanitize(true), Ok(())); assert_eq!(tx.message.account_keys.len(), 3); tx = o.clone(); tx.message.header.num_required_signatures = 3; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.header.num_readonly_signed_accounts = 4; tx.message.header.num_readonly_unsigned_accounts = 0; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.header.num_readonly_signed_accounts = 2; tx.message.header.num_readonly_unsigned_accounts = 2; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.header.num_readonly_signed_accounts = 0; tx.message.header.num_readonly_unsigned_accounts = 4; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.instructions[0].program_id_index = 3; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.instructions[0].accounts[0] = 3; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.instructions[0].program_id_index = 0; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o.clone(); tx.message.header.num_readonly_signed_accounts = 2; tx.message.header.num_readonly_unsigned_accounts = 3; tx.message.account_keys.resize(4, Address::default()); - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); tx = o; tx.message.header.num_readonly_signed_accounts = 2; tx.message.header.num_required_signatures = 1; - assert_eq!(tx.sanitize(), Err(SanitizeError::IndexOutOfBounds)); + assert_eq!(tx.sanitize(true), Err(SanitizeError::IndexOutOfBounds)); } fn create_sample_transaction() -> Transaction { diff --git a/transaction/src/sanitized.rs b/transaction/src/sanitized.rs index 66375c47e..ecf3f0ff5 100644 --- a/transaction/src/sanitized.rs +++ b/transaction/src/sanitized.rs @@ -122,8 +122,9 @@ impl SanitizedTransaction { pub fn try_from_legacy_transaction( tx: Transaction, reserved_account_keys: &HashSet
, + limit_ix_accounts_simd_406: bool, ) -> TransactionResult { - tx.sanitize()?; + tx.sanitize(limit_ix_accounts_simd_406)?; Ok(Self { message_hash: tx.message.hash(), @@ -140,7 +141,7 @@ impl SanitizedTransaction { #[cfg(feature = "blake3")] pub fn from_transaction_for_tests(tx: Transaction) -> Self { let empty_key_set = HashSet::default(); - Self::try_from_legacy_transaction(tx, &empty_key_set).unwrap() + Self::try_from_legacy_transaction(tx, &empty_key_set, true).unwrap() } /// Create a sanitized transaction from fields. @@ -410,6 +411,7 @@ mod tests { ..legacy::Message::default() }, &HashSet::default(), + true, ) .unwrap(); diff --git a/transaction/src/versioned/mod.rs b/transaction/src/versioned/mod.rs index 31146a8e7..6d6518120 100644 --- a/transaction/src/versioned/mod.rs +++ b/transaction/src/versioned/mod.rs @@ -121,8 +121,11 @@ impl VersionedTransaction { }) } - pub fn sanitize(&self) -> std::result::Result<(), SanitizeError> { - self.message.sanitize()?; + pub fn sanitize( + &self, + limit_ix_accounts_simd_406: bool, + ) -> std::result::Result<(), SanitizeError> { + self.message.sanitize(limit_ix_accounts_simd_406)?; self.sanitize_signatures()?; Ok(()) }