From 0542441b532b50cf063cff01030c7e73a3df46af Mon Sep 17 00:00:00 2001 From: Colin Finck Date: Mon, 4 Oct 2021 22:49:07 +0200 Subject: Add more size and range checks with proper error handling. This crate must never panic on invalid input data! --- src/boot_sector.rs | 7 +++- src/error.rs | 13 ++++++++ src/index.rs | 14 ++++---- src/index_entry.rs | 98 +++++++++++++++++++++++++++++++++++++++--------------- 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 { /// 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, + 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 // 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::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 { 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> @@ -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 { + pub fn subnode_vcn(&self) -> Option> { 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::(); - 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::(), + INDEX_ENTRY_HEADER_SIZE, + ); + let end = start + mem::size_of::(); + + 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 Iterator for IndexNodeEntryRanges where E: NtfsIndexEntryType, { - type Item = IndexEntryRange; + type Item = Result>; fn next(&mut self) -> Option { 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::::new(&self.data[start..], position); + let entry = iter_try!(NtfsIndexEntry::::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>; fn next(&mut self) -> Option { 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)) } } -- cgit v1.2.3