Add more size and range checks with proper error handling.

This crate must never panic on invalid input data!

4 files changed, 98 insertions, 34 deletions

diff --git a/src/boot_sector.rs b/src/boot_sector.rs
index 0132d9e..f443665 100644
--- a/src/boot_sector.rs
+++ b/src/boot_sector.rs
@@ -68,6 +68,10 @@ impl BiosParameterBlock {
     /// Source: https://en.wikipedia.org/wiki/NTFS#Partition_Boot_Sector_(VBR)
     fn record_size(&self, size_info: i8) -> Result<u32> {
         /// The usual exponent of `BiosParameterBlock::file_record_size_info` is 10 (2^10 = 1024 bytes).
+        /// Exponents < 10 have never been seen and are denied to guarantee that every record header
+        /// fits into a record.
+        const MINIMUM_SIZE_INFO_EXPONENT: u32 = 10;
+
         /// Exponents > 10 would come as a surprise, but our code should still be able to handle those.
         /// Exponents > 31 (2^31 = 2 GiB) would make no sense, exceed a u32, and must be outright denied.
         const MAXIMUM_SIZE_INFO_EXPONENT: u32 = 31;
@@ -85,7 +89,8 @@ impl BiosParameterBlock {
         } else {
             // The size field denotes a binary exponent after negation.
             let exponent = (-size_info) as u32;
-            if exponent >= MAXIMUM_SIZE_INFO_EXPONENT {
+
+            if exponent < MINIMUM_SIZE_INFO_EXPONENT || exponent >= MAXIMUM_SIZE_INFO_EXPONENT {
                 return Err(NtfsError::InvalidRecordSizeInfo {
                     size_info,
                     cluster_size,
diff --git a/src/error.rs b/src/error.rs
index a0b6901..5bb202d 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,6 +3,7 @@
 
 use crate::attribute::NtfsAttributeType;
 use crate::types::{Lcn, Vcn};
+use core::ops::Range;
 use displaydoc::Display;
 
 /// Central result type of ntfs.
@@ -71,6 +72,18 @@ pub enum NtfsError {
         expected: u32,
         actual: u32,
     },
+    /// The NTFS index entry at byte position {position:#010x} references a data field in the range {range:?}, but the entry only has a size of {size} bytes
+    InvalidIndexEntryDataRange {
+        position: u64,
+        range: Range<usize>,
+        size: u16,
+    },
+    /// The NTFS index entry at byte position {position:#010x} reports a size of {expected} bytes, but it only has {actual} bytes
+    InvalidIndexEntrySize {
+        position: u64,
+        expected: u16,
+        actual: u16,
+    },
     /// The NTFS index root at byte position {position:#010x} indicates that its entries start at offset {expected}, but the index root only has a size of {actual} bytes
     InvalidIndexRootEntriesOffset {
         position: u64,
diff --git a/src/index.rs b/src/index.rs
index 9239371..2fd4330 100644
--- a/src/index.rs
+++ b/src/index.rs
@@ -127,12 +127,14 @@ where
 
             // Get the next `IndexEntryRange` from it.
             if let Some(entry_range) = iter.next() {
+                let entry_range = iter_try!(entry_range);
                 // Convert that `IndexEntryRange` to a (lifetime-bound) `NtfsIndexEntry`.
-                let entry = entry_range.to_entry(iter.data());
+                let entry = iter_try!(entry_range.to_entry(iter.data()));
                 let is_last_entry = entry.flags().contains(NtfsIndexEntryFlags::LAST_ENTRY);
 
                 // Does this entry have a subnode that needs to be iterated first?
                 if let Some(subnode_vcn) = entry.subnode_vcn() {
+                    let subnode_vcn = iter_try!(subnode_vcn);
                     // Read the subnode from the filesystem and get an iterator for it.
                     let index_allocation_item =
                         iter_try!(self.index.index_allocation_item.as_ref().ok_or_else(|| {
@@ -181,7 +183,7 @@ where
         };
 
         let iter = self.inner_iterators.last().unwrap();
-        let entry = entry_range.to_entry(iter.data());
+        let entry = iter_try!(entry_range.to_entry(iter.data()));
         Some(Ok(entry))
     }
 }
@@ -227,8 +229,8 @@ where
             //
             // A textbook B-tree search algorithm would get the middle entry and perform binary search.
             // But we can't do that here, as we are dealing with variable-length entries.
-            let entry_range = self.inner_iterator.next()?;
-            let entry = entry_range.to_entry(self.inner_iterator.data());
+            let entry_range = iter_try!(self.inner_iterator.next()?);
+            let entry = iter_try!(entry_range.to_entry(self.inner_iterator.data()));
 
             // Check if this entry has a key.
             if let Some(key) = entry.key() {
@@ -239,7 +241,7 @@ where
                     Ordering::Equal => {
                         // We found what we were looking for!
                         // Recreate `entry` from the last `self.inner_iterator` to please the borrow checker.
-                        let entry = entry_range.to_entry(self.inner_iterator.data());
+                        let entry = iter_try!(entry_range.to_entry(self.inner_iterator.data()));
                         return Some(Ok(entry));
                     }
                     Ordering::Less => {
@@ -257,7 +259,7 @@ where
             // Either this entry has no key (= is the last one on this subnode level) or
             // it comes lexicographically AFTER what we're looking for.
             // In both cases, we have to continue iterating in the subnode of this entry (if there is any).
-            let subnode_vcn = entry.subnode_vcn()?;
+            let subnode_vcn = iter_try!(entry.subnode_vcn()?);
             let index_allocation_item =
                 iter_try!(self.index.index_allocation_item.as_ref().ok_or_else(|| {
                     NtfsError::MissingIndexAllocation {
diff --git a/src/index_entry.rs b/src/index_entry.rs
index b6fcb55..c9f0805 100644
--- a/src/index_entry.rs
+++ b/src/index_entry.rs
@@ -1,7 +1,7 @@
 // Copyright 2021 Colin Finck <colin@reactos.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use crate::error::Result;
+use crate::error::{NtfsError, Result};
 use crate::file::NtfsFile;
 use crate::file_reference::NtfsFileReference;
 use crate::indexes::{
@@ -21,7 +21,7 @@ use core::ops::Range;
 use memoffset::offset_of;
 
 /// Size of all [`IndexEntryHeader`] fields plus some reserved bytes.
-const INDEX_ENTRY_HEADER_SIZE: i64 = 16;
+const INDEX_ENTRY_HEADER_SIZE: usize = 16;
 
 #[repr(C, packed)]
 struct IndexEntryHeader {
@@ -70,7 +70,7 @@ where
         }
     }
 
-    pub(crate) fn to_entry<'s>(&self, slice: &'s [u8]) -> NtfsIndexEntry<'s, E> {
+    pub(crate) fn to_entry<'s>(&self, slice: &'s [u8]) -> Result<NtfsIndexEntry<'s, E>> {
         NtfsIndexEntry::new(&slice[self.range.clone()], self.position)
     }
 }
@@ -89,13 +89,18 @@ impl<'s, E> NtfsIndexEntry<'s, E>
 where
     E: NtfsIndexEntryType,
 {
-    pub(crate) fn new(slice: &'s [u8], position: u64) -> Self {
+    pub(crate) fn new(slice: &'s [u8], position: u64) -> Result<Self> {
         let entry_type = PhantomData;
-        Self {
+
+        let mut entry = Self {
             slice,
             position,
             entry_type,
-        }
+        };
+        entry.validate_size()?;
+        entry.slice = &entry.slice[..entry.index_entry_length() as usize];
+
+        Ok(entry)
     }
 
     pub fn data(&self) -> Option<Result<E::DataType>>
@@ -110,10 +115,14 @@ where
         let end = start + self.data_length() as usize;
         let position = self.position + start as u64;
 
-        let data = iter_try!(E::DataType::data_from_slice(
-            &self.slice[start..end],
-            position
-        ));
+        let slice = self.slice.get(start..end);
+        let slice = iter_try!(slice.ok_or(NtfsError::InvalidIndexEntryDataRange {
+            position: self.position,
+            range: start..end,
+            size: self.slice.len() as u16
+        }));
+
+        let data = iter_try!(E::DataType::data_from_slice(slice, position));
         Some(Ok(data))
     }
 
@@ -163,14 +172,18 @@ where
             return None;
         }
 
-        let start = INDEX_ENTRY_HEADER_SIZE as usize;
+        let start = INDEX_ENTRY_HEADER_SIZE;
         let end = start + self.key_length() as usize;
         let position = self.position + start as u64;
 
-        let key = iter_try!(E::KeyType::key_from_slice(
-            &self.slice[start..end],
-            position
-        ));
+        let slice = self.slice.get(start..end);
+        let slice = iter_try!(slice.ok_or(NtfsError::InvalidIndexEntryDataRange {
+            position: self.position,
+            range: start..end,
+            size: self.slice.len() as u16
+        }));
+
+        let key = iter_try!(E::KeyType::key_from_slice(slice, position));
         Some(Ok(key))
     }
 
@@ -181,16 +194,27 @@ where
 
     /// Returns the Virtual Cluster Number (VCN) of the subnode of this Index Entry,
     /// or `None` if this Index Entry has no subnode.
-    pub fn subnode_vcn(&self) -> Option<Vcn> {
+    pub fn subnode_vcn(&self) -> Option<Result<Vcn>> {
         if !self.flags().contains(NtfsIndexEntryFlags::HAS_SUBNODE) {
             return None;
         }
 
-        // Get the subnode VCN from the very end of the Index Entry.
-        let start = self.index_entry_length() as usize - mem::size_of::<Vcn>();
-        let vcn = Vcn::from(LittleEndian::read_i64(&self.slice[start..]));
-
-        Some(vcn)
+        // Get the subnode VCN from the very end of the Index Entry, but at least after the header.
+        let start = usize::max(
+            self.index_entry_length() as usize - mem::size_of::<Vcn>(),
+            INDEX_ENTRY_HEADER_SIZE,
+        );
+        let end = start + mem::size_of::<Vcn>();
+
+        let slice = self.slice.get(start..end);
+        let slice = iter_try!(slice.ok_or(NtfsError::InvalidIndexEntryDataRange {
+            position: self.position,
+            range: start..end,
+            size: self.slice.len() as u16
+        }));
+
+        let vcn = Vcn::from(LittleEndian::read_i64(slice));
+        Some(Ok(vcn))
     }
 
     /// Returns an [`NtfsFile`] for the file referenced by this index entry.
@@ -201,6 +225,26 @@ where
     {
         self.file_reference().to_file(ntfs, fs)
     }
+
+    fn validate_size(&self) -> Result<()> {
+        if self.slice.len() < INDEX_ENTRY_HEADER_SIZE {
+            return Err(NtfsError::InvalidIndexEntrySize {
+                position: self.position,
+                expected: INDEX_ENTRY_HEADER_SIZE as u16,
+                actual: self.slice.len() as u16,
+            });
+        }
+
+        if self.index_entry_length() as usize > self.slice.len() {
+            return Err(NtfsError::InvalidIndexEntrySize {
+                position: self.position,
+                expected: self.index_entry_length(),
+                actual: self.slice.len() as u16,
+            });
+        }
+
+        Ok(())
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -239,7 +283,7 @@ impl<E> Iterator for IndexNodeEntryRanges<E>
 where
     E: NtfsIndexEntryType,
 {
-    type Item = IndexEntryRange<E>;
+    type Item = Result<IndexEntryRange<E>>;
 
     fn next(&mut self) -> Option<Self::Item> {
         if self.range.is_empty() {
@@ -249,7 +293,7 @@ where
         // Get the current entry.
         let start = self.range.start;
         let position = self.position;
-        let entry = NtfsIndexEntry::<E>::new(&self.data[start..], position);
+        let entry = iter_try!(NtfsIndexEntry::<E>::new(&self.data[start..], position));
         let end = start + entry.index_entry_length() as usize;
 
         if entry.flags().contains(NtfsIndexEntryFlags::LAST_ENTRY) {
@@ -263,7 +307,7 @@ where
             self.position += entry.index_entry_length() as u64;
         }
 
-        Some(IndexEntryRange::new(start..end, position))
+        Some(Ok(IndexEntryRange::new(start..end, position)))
     }
 }
 
@@ -297,7 +341,7 @@ impl<'s, E> Iterator for NtfsIndexNodeEntries<'s, E>
 where
     E: NtfsIndexEntryType,
 {
-    type Item = NtfsIndexEntry<'s, E>;
+    type Item = Result<NtfsIndexEntry<'s, E>>;
 
     fn next(&mut self) -> Option<Self::Item> {
         if self.slice.is_empty() {
@@ -305,7 +349,7 @@ where
         }
 
         // Get the current entry.
-        let entry = NtfsIndexEntry::new(self.slice, self.position);
+        let entry = iter_try!(NtfsIndexEntry::new(self.slice, self.position));
 
         if entry.flags().contains(NtfsIndexEntryFlags::LAST_ENTRY) {
             // This is the last entry.
@@ -319,7 +363,7 @@ where
             self.position += bytes_to_advance as u64;
         }
 
-        Some(entry)
+        Some(Ok(entry))
     }
 }