avoid reading entry data as num_entries when offset > 0

Author: rustopian

sdk/pinocchio/src/sysvars/slot_hashes/raw.rs

@@ -11,27 +11,35 @@ use super::*;
 /// Validates that a buffer is properly sized for `SlotHashes` data.
 ///
 /// Checks that the buffer length is `8 + (N × 40)` for some `N ≤ 512`.
-/// Unlike the `SlotHashes` constructor, this function does not require
-/// the buffer to be exactly 20,488 bytes.
+///
+/// This returns the maximum number of entries that can fit in the buffer.
 #[inline(always)]
-pub(crate) fn validate_buffer_size(buffer_len: usize) -> Result<(), ProgramError> {
-    // Must have space for 8-byte header
-    if buffer_len < NUM_ENTRIES_SIZE {
-        return Err(ProgramError::AccountDataTooSmall);
-    }
+pub(crate) fn validate_buffer_size(
+    buffer_len: usize,
+    offset: usize,
+) -> Result<usize, ProgramError> {
+    let data_len = if offset == 0 {
+        if buffer_len == MAX_SIZE {
+            return Ok(MAX_ENTRIES);
+        }
+        // Must have space for 8-byte header
+        if buffer_len < NUM_ENTRIES_SIZE {
+            return Err(ProgramError::AccountDataTooSmall);
+        }
+        buffer_len - NUM_ENTRIES_SIZE
+    } else {
+        buffer_len
+    };
 
-    // Calculate how many entries can fit
-    let data_len = buffer_len - NUM_ENTRIES_SIZE;
     if data_len % ENTRY_SIZE != 0 {
         return Err(ProgramError::InvalidArgument);
     }
-
-    let max_entries = data_len / ENTRY_SIZE;
-    if max_entries > MAX_ENTRIES {
+    let max_entries_in_buffer = data_len / ENTRY_SIZE;
+    if max_entries_in_buffer > MAX_ENTRIES {
         return Err(ProgramError::InvalidArgument);
     }
 
-    Ok(())
+    Ok(max_entries_in_buffer)
 }
 
 /// Validates offset parameters for fetching `SlotHashes` data.
@@ -55,26 +63,26 @@ pub fn validate_fetch_offset(offset: usize, buffer_len: usize) -> Result<(), Pro
 
 /// Copies `SlotHashes` sysvar bytes into `buffer`, performing validation.
 ///
-/// Returns the number of entries present in the sysvar.
+/// Returns the number of entries *that can fit in the buffer*. If the buffer
+/// can fit more entries than are present in the sysvar from `offset` to the end,
+/// this returned value will mismatch the actual entries filled into the buffer.
+/// When `offset > 0`, it is recommended not to provide buffers larger than
+/// `MAX_SIZE - NUM_ENTRIES_SIZE - (offset * ENTRY_SIZE)`.
 #[inline(always)]
 pub fn fetch_into(buffer: &mut [u8], offset: usize) -> Result<usize, ProgramError> {
-    if buffer.len() != MAX_SIZE {
-        validate_buffer_size(buffer.len())?;
-    }
+    let num_entries = validate_buffer_size(buffer.len(), offset)?;
 
     validate_fetch_offset(offset, buffer.len())?;
 
     // SAFETY: `buffer.len()` and `offset` are both validated above.
     unsafe { fetch_into_unchecked(buffer, offset) }?;
 
-    let num_entries = read_entry_count_from_bytes(buffer).unwrap_or(0);
-
-    let required_len = NUM_ENTRIES_SIZE + num_entries * ENTRY_SIZE;
-    if buffer.len() < required_len {
-        return Err(ProgramError::InvalidArgument);
+    if offset == 0 {
+        // If header is preset, read entries from it
+        Ok(read_entry_count_from_bytes(buffer).unwrap_or(0))
+    } else {
+        Ok(num_entries)
     }
-
-    Ok(num_entries)
 }
 
 /// Copies `SlotHashes` sysvar bytes into `buffer` **without** validation.

sdk/pinocchio/src/sysvars/slot_hashes/test_raw.rs

@@ -6,27 +6,74 @@ extern crate std;
 
 #[test]
 fn test_validate_buffer_size() {
+    // ===== Tests with offset = 0 (buffer includes header) =====
+
+    // Too small to fit header
     let small_len = 4;
-    assert!(raw::validate_buffer_size(small_len).is_err());
+    assert!(raw::validate_buffer_size(small_len, 0).is_err());
 
+    // Misaligned: header + partial entry
     let misaligned_len = NUM_ENTRIES_SIZE + 39;
-    assert!(raw::validate_buffer_size(misaligned_len).is_err());
+    assert!(raw::validate_buffer_size(misaligned_len, 0).is_err());
 
+    // Too large: more than MAX_ENTRIES
     let oversized_len = NUM_ENTRIES_SIZE + (MAX_ENTRIES + 1) * ENTRY_SIZE;
-    assert!(raw::validate_buffer_size(oversized_len).is_err());
+    assert!(raw::validate_buffer_size(oversized_len, 0).is_err());
 
+    // Valid cases with offset = 0
     let valid_empty_len = NUM_ENTRIES_SIZE;
-    assert!(raw::validate_buffer_size(valid_empty_len).is_ok());
+    assert_eq!(raw::validate_buffer_size(valid_empty_len, 0).unwrap(), 0);
 
     let valid_one_len = NUM_ENTRIES_SIZE + ENTRY_SIZE;
-    assert!(raw::validate_buffer_size(valid_one_len).is_ok());
+    assert_eq!(raw::validate_buffer_size(valid_one_len, 0).unwrap(), 1);
 
     let valid_max_len = NUM_ENTRIES_SIZE + MAX_ENTRIES * ENTRY_SIZE;
-    assert!(raw::validate_buffer_size(valid_max_len).is_ok());
+    assert_eq!(
+        raw::validate_buffer_size(valid_max_len, 0).unwrap(),
+        MAX_ENTRIES
+    );
+
+    // Edge case: exactly at the boundary (MAX_SIZE)
+    assert_eq!(raw::validate_buffer_size(MAX_SIZE, 0).unwrap(), MAX_ENTRIES);
+
+    // ===== Tests with offset != 0 (buffer doesn't include header) =====
+
+    // Valid cases with non-zero offset - buffer contains only entry data
+
+    // Buffer for exactly 1 entry
+    assert_eq!(raw::validate_buffer_size(ENTRY_SIZE, 8).unwrap(), 1);
+
+    // Buffer for exactly 2 entries
+    assert_eq!(raw::validate_buffer_size(2 * ENTRY_SIZE, 8).unwrap(), 2);
+
+    // Buffer for maximum entries (without header space)
+    assert_eq!(
+        raw::validate_buffer_size(MAX_ENTRIES * ENTRY_SIZE, 8).unwrap(),
+        MAX_ENTRIES
+    );
+
+    // Buffer for 10 entries
+    assert_eq!(raw::validate_buffer_size(10 * ENTRY_SIZE, 48).unwrap(), 10);
 
-    // Edge case: exactly at the boundary
-    let boundary_len = NUM_ENTRIES_SIZE + MAX_ENTRIES * ENTRY_SIZE;
-    assert!(raw::validate_buffer_size(boundary_len).is_ok());
+    // Error cases with non-zero offset
+
+    // Misaligned buffer - not a multiple of ENTRY_SIZE
+    assert!(raw::validate_buffer_size(ENTRY_SIZE + 1, 8).is_err());
+    assert!(raw::validate_buffer_size(ENTRY_SIZE - 1, 8).is_err());
+    assert!(raw::validate_buffer_size(39, 8).is_err()); // 39 is not divisible by 40
+
+    // Too many entries - more than MAX_ENTRIES
+    assert!(raw::validate_buffer_size((MAX_ENTRIES + 1) * ENTRY_SIZE, 8).is_err());
+    assert!(raw::validate_buffer_size((MAX_ENTRIES + 10) * ENTRY_SIZE, 48).is_err());
+
+    // Empty buffer with offset (valid - 0 entries)
+    assert_eq!(raw::validate_buffer_size(0, 8).unwrap(), 0);
+
+    // ===== Additional edge cases =====
+
+    // Large offset values (should still work for buffer size validation)
+    assert_eq!(raw::validate_buffer_size(5 * ENTRY_SIZE, 1000).unwrap(), 5);
+    assert!(raw::validate_buffer_size(5 * ENTRY_SIZE + 1, 2000).is_err()); // misaligned
 }
 
 #[test]
@@ -90,9 +137,10 @@ fn test_fetch_into_host_stub() {
     let n3 = raw::fetch_into(&mut one_entry, 0).expect("fetch_into(one_entry, 0)");
     assert_eq!(n3, 0);
 
-    // 4. Header-skipped fetch should fail because header is missing.
+    // 4. Header-skipped fetch should succeed and return the number of entries that fit.
     let mut skip_header = std::vec![0u8; ENTRY_SIZE];
-    assert!(raw::fetch_into(&mut skip_header, 8).is_err());
+    let entries_count = raw::fetch_into(&mut skip_header, 8).expect("fetch_into(skip_header, 8)");
+    assert_eq!(entries_count, 1); // Buffer can fit exactly 1 entry
 
     // 5. Mis-aligned buffer size should error.
     let mut misaligned = std::vec![0u8; NUM_ENTRIES_SIZE + 39];
@@ -106,3 +154,32 @@ fn test_fetch_into_host_stub() {
     let mut small = std::vec![0u8; 200];
     assert!(raw::fetch_into(&mut small, MAX_SIZE - 199).is_err());
 }
+
+/// Test that `fetch_into` with offset correctly avoids interpreting slot
+/// data as entry count.
+#[cfg(test)]
+#[test]
+fn test_fetch_into_offset_avoids_incorrect_entry_count() {
+    // The core issue: when fetch_into is called with offset != 0, the first
+    // 8 bytes of the buffer contains header data, NOT entry data.
+    let mut buffer = std::vec![0u8; 3 * ENTRY_SIZE];
+
+    // Call fetch_into with offset 8 (skipping the 8-byte header)
+    let result = raw::fetch_into(&mut buffer, 8);
+
+    assert!(
+        result.is_ok(),
+        "fetch_into should succeed with offset that skips header"
+    );
+
+    let entries_that_fit = result.unwrap();
+    assert_eq!(
+        entries_that_fit, 3,
+        "Should return number of entries that fit in buffer, not some slot number"
+    );
+
+    // Buffer for exactly 1 entry starting from offset 48 (2nd entry)
+    let mut second_entry_buffer = std::vec![0u8; ENTRY_SIZE];
+    let second_result = raw::fetch_into(&mut second_entry_buffer, 48).unwrap();
+    assert_eq!(second_result, 1);
+}