TransactionView - sanitization checks (#2757)

transaction-view/benches/transaction_view.rs

@@ -15,7 +15,7 @@ use {
         signature::Keypair,
         signer::Signer,
         system_instruction,
-        transaction::VersionedTransaction,
+        transaction::{SanitizedVersionedTransaction, VersionedTransaction},
     },
 };
 
@@ -41,11 +41,30 @@ fn bench_transactions_parsing(
         });
     });
 
+    // Legacy Transaction Parsing and Sanitize checks
+    group.bench_function("SanitizedVersionedTransaction", |c| {
+        c.iter(|| {
+            for bytes in serialized_transactions.iter() {
+                let tx = bincode::deserialize::<VersionedTransaction>(black_box(bytes)).unwrap();
+                let _ = SanitizedVersionedTransaction::try_new(tx).unwrap();
+            }
+        });
+    });
+
     // New Transaction Parsing
     group.bench_function("TransactionView", |c| {
         c.iter(|| {
             for bytes in serialized_transactions.iter() {
-                let _ = TransactionView::try_new(black_box(bytes.as_ref())).unwrap();
+                let _ = TransactionView::try_new_unsanitized(black_box(bytes.as_ref())).unwrap();
+            }
+        });
+    });
+
+    // New Transaction Parsing and Sanitize checks
+    group.bench_function("TransactionView (Sanitized)", |c| {
+        c.iter(|| {
+            for bytes in serialized_transactions.iter() {
+                let _ = TransactionView::try_new_sanitized(black_box(bytes.as_ref())).unwrap();
             }
         });
     });

transaction-view/src/address_table_lookup_meta.rs

@@ -4,7 +4,7 @@ use {
             advance_offset_for_array, advance_offset_for_type, check_remaining,
             optimized_read_compressed_u16, read_byte, read_slice_data, read_type,
         },
-        result::{Result, TransactionParsingError},
+        result::{Result, TransactionViewError},
     },
     solana_sdk::{hash::Hash, packet::PACKET_DATA_SIZE, pubkey::Pubkey, signature::Signature},
     solana_svm_transaction::message_address_table_lookup::SVMMessageAddressTableLookup,
@@ -51,6 +51,10 @@ pub(crate) struct AddressTableLookupMeta {
     pub(crate) num_address_table_lookups: u8,
     /// The offset to the first address table lookup in the transaction.
     pub(crate) offset: u16,
+    /// The total number of writable lookup accounts in the transaction.
+    pub(crate) total_writable_lookup_accounts: u16,
+    /// The total number of readonly lookup accounts in the transaction.
+    pub(crate) total_readonly_lookup_accounts: u16,
 }
 
 impl AddressTableLookupMeta {
@@ -66,7 +70,7 @@ impl AddressTableLookupMeta {
         const _: () = assert!(MAX_ATLS_PER_PACKET & 0b1000_0000 == 0);
         let num_address_table_lookups = read_byte(bytes, offset)?;
         if num_address_table_lookups > MAX_ATLS_PER_PACKET {
-            return Err(TransactionParsingError);
+            return Err(TransactionViewError::ParseError);
         }
 
         // Check that the remaining bytes are enough to hold the ATLs.
@@ -80,6 +84,13 @@ impl AddressTableLookupMeta {
         // length is less than u16::MAX, so we can safely cast to u16.
         let address_table_lookups_offset = *offset as u16;
 
+        // Check that there is no chance of overflow when calculating the total
+        // number of writable and readonly lookup accounts using a u32.
+        const _: () =
+            assert!(u16::MAX as usize * MAX_ATLS_PER_PACKET as usize <= u32::MAX as usize);
+        let mut total_writable_lookup_accounts: u32 = 0;
+        let mut total_readonly_lookup_accounts: u32 = 0;
+
         // The ATLs do not have a fixed size. So we must iterate over
         // each ATL to find the total size of the ATLs in the packet,
         // and check for any malformed ATLs or buffer overflows.
@@ -94,16 +105,24 @@ impl AddressTableLookupMeta {
 
             // Read the number of write indexes, and then update the offset.
             let num_write_accounts = optimized_read_compressed_u16(bytes, offset)?;
+            total_writable_lookup_accounts =
+                total_writable_lookup_accounts.wrapping_add(u32::from(num_write_accounts));
             advance_offset_for_array::<u8>(bytes, offset, num_write_accounts)?;
 
             // Read the number of read indexes, and then update the offset.
             let num_read_accounts = optimized_read_compressed_u16(bytes, offset)?;
-            advance_offset_for_array::<u8>(bytes, offset, num_read_accounts)?
+            total_readonly_lookup_accounts =
+                total_readonly_lookup_accounts.wrapping_add(u32::from(num_read_accounts));
+            advance_offset_for_array::<u8>(bytes, offset, num_read_accounts)?;
         }
 
         Ok(Self {
             num_address_table_lookups,
             offset: address_table_lookups_offset,
+            total_writable_lookup_accounts: u16::try_from(total_writable_lookup_accounts)
+                .map_err(|_| TransactionViewError::SanitizeError)?,
+            total_readonly_lookup_accounts: u16::try_from(total_readonly_lookup_accounts)
+                .map_err(|_| TransactionViewError::SanitizeError)?,
         })
     }
 }
@@ -195,6 +214,8 @@ mod tests {
         assert_eq!(meta.num_address_table_lookups, 0);
         assert_eq!(meta.offset, 1);
         assert_eq!(offset, bytes.len());
+        assert_eq!(meta.total_writable_lookup_accounts, 0);
+        assert_eq!(meta.total_readonly_lookup_accounts, 0);
     }
 
     #[test]
@@ -221,6 +242,8 @@ mod tests {
         assert_eq!(meta.num_address_table_lookups, 1);
         assert_eq!(meta.offset, 1);
         assert_eq!(offset, bytes.len());
+        assert_eq!(meta.total_writable_lookup_accounts, 3);
+        assert_eq!(meta.total_readonly_lookup_accounts, 3);
     }
 
     #[test]
@@ -234,7 +257,7 @@ mod tests {
             MessageAddressTableLookup {
                 account_key: Pubkey::new_unique(),
                 writable_indexes: vec![1, 2, 3],
-                readonly_indexes: vec![4, 5, 6],
+                readonly_indexes: vec![4, 5],
             },
         ]))
         .unwrap();
@@ -243,6 +266,8 @@ mod tests {
         assert_eq!(meta.num_address_table_lookups, 2);
         assert_eq!(meta.offset, 1);
         assert_eq!(offset, bytes.len());
+        assert_eq!(meta.total_writable_lookup_accounts, 6);
+        assert_eq!(meta.total_readonly_lookup_accounts, 5);
     }
 
     #[test]

transaction-view/src/bytes.rs

@@ -1,4 +1,4 @@
-use crate::result::{Result, TransactionParsingError};
+use crate::result::{Result, TransactionViewError};
 
 /// Check that the buffer has at least `len` bytes remaining starting at
 /// `offset`. Returns Err if the buffer is too short.
@@ -12,7 +12,7 @@ use crate::result::{Result, TransactionParsingError};
 #[inline(always)]
 pub fn check_remaining(bytes: &[u8], offset: usize, num_bytes: usize) -> Result<()> {
     if num_bytes > bytes.len().wrapping_sub(offset) {
-        Err(TransactionParsingError)
+        Err(TransactionViewError::ParseError)
     } else {
         Ok(())
     }
@@ -24,7 +24,10 @@ pub fn check_remaining(bytes: &[u8], offset: usize, num_bytes: usize) -> Result<
 pub fn read_byte(bytes: &[u8], offset: &mut usize) -> Result<u8> {
     // Implicitly checks that the offset is within bounds, no need
     // to call `check_remaining` explicitly here.
-    let value = bytes.get(*offset).copied().ok_or(TransactionParsingError);
+    let value = bytes
+        .get(*offset)
+        .copied()
+        .ok_or(TransactionViewError::ParseError);
     *offset = offset.wrapping_add(1);
     value
 }
@@ -49,10 +52,10 @@ pub fn read_compressed_u16(bytes: &[u8], offset: &mut usize) -> Result<u16> {
         // to call check_remaining explicitly here.
         let byte = *bytes
             .get(offset.wrapping_add(i))
-            .ok_or(TransactionParsingError)?;
+            .ok_or(TransactionViewError::ParseError)?;
         // non-minimal encoding or overflow
         if (i > 0 && byte == 0) || (i == 2 && byte > 3) {
-            return Err(TransactionParsingError);
+            return Err(TransactionViewError::ParseError);
         }
         result |= ((byte & 0x7F) as u16) << shift;
         shift = shift.wrapping_add(7);
@@ -86,7 +89,7 @@ pub fn optimized_read_compressed_u16(bytes: &[u8], offset: &mut usize) -> Result
     let mut result = 0u16;
 
     // First byte
-    let byte1 = *bytes.get(*offset).ok_or(TransactionParsingError)?;
+    let byte1 = *bytes.get(*offset).ok_or(TransactionViewError::ParseError)?;
     result |= (byte1 & 0x7F) as u16;
     if byte1 & 0x80 == 0 {
         *offset = offset.wrapping_add(1);
@@ -96,9 +99,9 @@ pub fn optimized_read_compressed_u16(bytes: &[u8], offset: &mut usize) -> Result
     // Second byte
     let byte2 = *bytes
         .get(offset.wrapping_add(1))
-        .ok_or(TransactionParsingError)?;
+        .ok_or(TransactionViewError::ParseError)?;
     if byte2 == 0 || byte2 & 0x80 != 0 {
-        return Err(TransactionParsingError); // non-minimal encoding or overflow
+        return Err(TransactionViewError::ParseError); // non-minimal encoding or overflow
     }
     result |= ((byte2 & 0x7F) as u16) << 7;
     *offset = offset.wrapping_add(2);

transaction-view/src/lib.rs

@@ -8,6 +8,7 @@ mod address_table_lookup_meta;
 mod instructions_meta;
 mod message_header_meta;
 pub mod result;
+mod sanitize;
 mod signature_meta;
 pub mod static_account_keys_meta;
 pub mod transaction_data;

transaction-view/src/message_header_meta.rs

@@ -1,7 +1,7 @@
 use {
     crate::{
         bytes::read_byte,
-        result::{Result, TransactionParsingError},
+        result::{Result, TransactionViewError},
     },
     solana_sdk::message::MESSAGE_VERSION_PREFIX,
 };
@@ -49,7 +49,7 @@ impl MessageHeaderMeta {
             let version = message_prefix & !MESSAGE_VERSION_PREFIX;
             match version {
                 0 => (TransactionVersion::V0, read_byte(bytes, offset)?),
-                _ => return Err(TransactionParsingError),
+                _ => return Err(TransactionViewError::ParseError),
             }
         } else {
             // Legacy transaction. The `message_prefix` that was just read is

transaction-view/src/result.rs

@@ -1,3 +1,8 @@
 #[derive(Debug, PartialEq, Eq)]
-pub struct TransactionParsingError;
-pub type Result<T> = core::result::Result<T, TransactionParsingError>; // no distinction between errors for now
+#[repr(u8)] // repr(u8) is used to ensure that the enum is represented as a single byte in memory.
+pub enum TransactionViewError {
+    ParseError,
+    SanitizeError,
+}
+
+pub type Result<T> = core::result::Result<T, TransactionViewError>;