accessor: adapt and restrict contents access

Add checks for split variant inputs when accessing the payload
contents via the accessor instance. Both cases, accessing via the
safe `contents` method and via the previousely unsafe
`open_contents_at_range` call are covered.

Reduce possible misuse by wrapping the current plain content range
into an opaque `ContentRange` type with an additional optional
payload reference field to check consistency between the payload
reference encoded in the metadata archive and the payload header'
found in the payload data archive.

Because of the additional type wrapping and the payload header check,
the `open_contents_at_range` is considered safe now, dropping the
previously unsafe implementation.
The corresponding interfaces have been adapted accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>

src/accessor/aio.rs

@@ -7,14 +7,13 @@
 use std::future::Future;
 use std::io;
 use std::mem;
-use std::ops::Range;
 use std::os::unix::fs::FileExt;
 use std::path::Path;
 use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 
-use crate::accessor::{self, cache::Cache, MaybeReady, ReadAt, ReadAtOperation};
+use crate::accessor::{self, cache::Cache, ContentRange, MaybeReady, ReadAt, ReadAtOperation};
 use crate::decoder::aio::Decoder;
 use crate::format::GoodbyeItem;
 use crate::util;
@@ -153,13 +152,16 @@ impl<T: Clone + ReadAt> Accessor<T> {
     ///
     /// This will provide a reader over an arbitrary range of the archive file, so unless this
     /// comes from a actual file entry data, the contents might not make much sense.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContents<T> {
-        FileContents {
-            inner: unsafe { self.inner.open_contents_at_range(range) },
+    pub async fn open_contents_at_range(
+        &self,
+        range: &ContentRange,
+    ) -> io::Result<FileContents<T>> {
+        Ok(FileContents {
+            inner: self.inner.open_contents_at_range(range).await?,
             at: 0,
             buffer: Vec::new(),
             future: None,
-        }
+        })
     }
 
     /// Following a hardlink.
@@ -235,7 +237,7 @@ impl<T: Clone + ReadAt> FileEntry<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         self.inner.content_range()
     }
 

src/accessor/mod.rs

@@ -17,7 +17,7 @@ use endian_trait::Endian;
 
 use crate::binary_tree_array;
 use crate::decoder::{self, DecoderImpl};
-use crate::format::{self, FormatVersion, GoodbyeItem};
+use crate::format::{self, FormatVersion, GoodbyeItem, PayloadRef};
 use crate::util;
 use crate::{Entry, EntryKind, PxarVariant};
 
@@ -54,6 +54,16 @@ impl EntryRangeInfo {
     }
 }
 
+/// Stores a content range to be accessed via the `Accessor` as well as the payload reference to
+/// perform consistency checks on payload references for archives accessed via split variant input.
+#[derive(Clone)]
+pub struct ContentRange {
+    // Range of the content
+    content: Range<u64>,
+    // Optional payload ref
+    payload_ref: Option<PayloadRef>,
+}
+
 /// awaitable version of `ReadAt`.
 async fn read_at<T>(input: &T, buf: &mut [u8], offset: u64) -> io::Result<usize>
 where
@@ -335,13 +345,12 @@ impl<T: Clone + ReadAt> AccessorImpl<T> {
         })
     }
 
-    /// Allow opening arbitrary contents from a specific range.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContentsImpl<T> {
-        if let Some((payload_input, _)) = &self.input.payload() {
-            FileContentsImpl::new(payload_input.clone(), range)
-        } else {
-            FileContentsImpl::new(self.input.archive().clone(), range)
-        }
+    /// Open contents at provided range
+    pub async fn open_contents_at_range(
+        &self,
+        range: &ContentRange,
+    ) -> io::Result<FileContentsImpl<T>> {
+        FileContentsImpl::new(&self.input, range).await
     }
 
     /// Following a hardlink breaks a couple of conventions we otherwise have, particularly we will
@@ -758,7 +767,7 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         match self.entry.kind {
             EntryKind::File { offset: None, .. } => {
                 io_bail!("cannot open file, reader provided no offset")
@@ -767,7 +776,10 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
                 size,
                 offset: Some(offset),
                 payload_offset: None,
-            } => Ok(Some(offset..(offset + size))),
+            } => Ok(Some(ContentRange {
+                content: offset..(offset + size),
+                payload_ref: None,
+            })),
             // Payload offset beats regular offset if some
             EntryKind::File {
                 size,
@@ -775,7 +787,13 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
                 payload_offset: Some(payload_offset),
             } => {
                 let start_offset = payload_offset + size_of::<format::Header>() as u64;
-                Ok(Some(start_offset..start_offset + size))
+                Ok(Some(ContentRange {
+                    content: start_offset..start_offset + size,
+                    payload_ref: Some(PayloadRef {
+                        offset: payload_offset,
+                        size,
+                    }),
+                }))
             }
             _ => Ok(None),
         }
@@ -785,11 +803,8 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
         let range = self
             .content_range()?
             .ok_or_else(|| io_format_err!("not a file"))?;
-        if let Some((ref payload_input, _)) = self.input.payload() {
-            Ok(FileContentsImpl::new(payload_input.clone(), range))
-        } else {
-            Ok(FileContentsImpl::new(self.input.archive().clone(), range))
-        }
+
+        FileContentsImpl::new(&self.input, &range).await
     }
 
     #[inline]
@@ -897,8 +912,25 @@ pub(crate) struct FileContentsImpl<T> {
 }
 
 impl<T: Clone + ReadAt> FileContentsImpl<T> {
-    pub fn new(input: T, range: Range<u64>) -> Self {
-        Self { input, range }
+    async fn new(
+        input: &PxarVariant<T, (T, Range<u64>)>,
+        range: &ContentRange,
+    ) -> io::Result<Self> {
+        let (input, range) = if let Some((payload_input, payload_range)) = input.payload() {
+            if let Some(payload_ref) = &range.payload_ref {
+                let header: format::Header =
+                    read_entry_at(payload_input, payload_ref.offset).await?;
+                format::check_payload_header_and_size(&header, payload_ref.size)?;
+            }
+            if payload_range.start > range.content.start || payload_range.end < range.content.end {
+                io_bail!("out of range access for payload");
+            }
+            (payload_input.clone(), range.content.clone())
+        } else {
+            (input.archive().clone(), range.content.clone())
+        };
+
+        Ok(Self { input, range })
     }
 
     #[inline]

src/accessor/sync.rs

@@ -1,14 +1,13 @@
 //! Blocking `pxar` random access handling.
 
 use std::io;
-use std::ops::Range;
 use std::os::unix::fs::FileExt;
 use std::path::Path;
 use std::pin::Pin;
 use std::sync::Arc;
 use std::task::Context;
 
-use crate::accessor::{self, cache::Cache, MaybeReady, ReadAt, ReadAtOperation};
+use crate::accessor::{self, cache::Cache, ContentRange, MaybeReady, ReadAt, ReadAtOperation};
 use crate::decoder::Decoder;
 use crate::format::GoodbyeItem;
 use crate::util::poll_result_once;
@@ -142,11 +141,14 @@ impl<T: Clone + ReadAt> Accessor<T> {
     ///
     /// This will provide a reader over an arbitrary range of the archive file, so unless this
     /// comes from a actual file entry data, the contents might not make much sense.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContents<T> {
-        FileContents {
-            inner: unsafe { self.inner.open_contents_at_range(range) },
+    pub unsafe fn open_contents_at_range(
+        &self,
+        range: &ContentRange,
+    ) -> io::Result<FileContents<T>> {
+        Ok(FileContents {
+            inner: poll_result_once(self.inner.open_contents_at_range(range))?,
             at: 0,
-        }
+        })
     }
 
     /// Following a hardlink.
@@ -291,7 +293,7 @@ impl<T: Clone + ReadAt> FileEntry<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         self.inner.content_range()
     }