]> git.proxmox.com Git - pxar.git/commitdiff
header: add size checks
authorFabian Grünbichler <f.gruenbichler@proxmox.com>
Fri, 12 Jun 2020 07:21:39 +0000 (09:21 +0200)
committerWolfgang Bumiller <w.bumiller@proxmox.com>
Fri, 12 Jun 2020 07:46:01 +0000 (09:46 +0200)
to catch
- headers with full_size < size_of::<Header>
- headers with content_size > header-specific limit

both should in practice only occur for corrupted streams/archives, but
panic-ing/attempting to allocate huge amounts of memory should be
avoided even for those.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
src/decoder/mod.rs
src/encoder/mod.rs
src/format/mod.rs
src/util.rs

index 960abe4eed0f66368071cf5b53756dc61ab472ad..22f069b0c6fce1d9a26731066319d6efa43d77c8 100644 (file)
@@ -350,6 +350,8 @@ impl<I: SeqRead> DecoderImpl<I> {
             Some(header) => header,
         };
 
+        header.check_header_size()?;
+
         if header.htype == format::PXAR_HARDLINK {
             // The only "dangling" header without an 'Entry' in front of it because it does not
             // carry its own metadata.
@@ -405,6 +407,8 @@ impl<I: SeqRead> DecoderImpl<I> {
             )
         };
         seq_read_exact(&mut self.input, dest).await?;
+        self.current_header.check_header_size()?;
+
         Ok(())
     }
 
index 3e1d7219621982f74b0d785ca40a12d7a18b1291..ad41ef81edf16048873bc715c5f59cc053ecca8d 100644 (file)
@@ -137,9 +137,12 @@ async fn seq_write_pxar_entry<T>(output: &mut T, htype: u64, data: &[u8]) -> io:
 where
     T: SeqWrite + ?Sized,
 {
+    let header = format::Header::with_content_size(htype, data.len() as u64);
+    header.check_header_size()?;
+
     seq_write_struct(
         &mut *output,
-        format::Header::with_content_size(htype, data.len() as u64),
+        header,
     )
     .await?;
     seq_write_all(output, data).await
@@ -151,9 +154,12 @@ async fn seq_write_pxar_entry_zero<T>(output: &mut T, htype: u64, data: &[u8]) -
 where
     T: SeqWrite + ?Sized,
 {
+    let header = format::Header::with_content_size(htype, 1 + data.len() as u64);
+    header.check_header_size()?;
+
     seq_write_struct(
         &mut *output,
-        format::Header::with_content_size(htype, 1 + data.len() as u64),
+        header,
     )
     .await?;
     seq_write_all(&mut *output, data).await?;
@@ -306,9 +312,12 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         let file_offset = seq_write_position(&mut self.output).await?;
         self.start_file_do(Some(metadata), file_name).await?;
 
+        let header = format::Header::with_content_size(format::PXAR_PAYLOAD, file_size);
+        header.check_header_size()?;
+
         seq_write_struct(
             &mut self.output,
-            format::Header::with_content_size(format::PXAR_PAYLOAD, file_size),
+            header,
         )
         .await?;
 
index 0a818e125abbcf93fde1676ab3b967708f039a91..57f7dc48c7d2d1a176418cdd7760c998df47f970 100644 (file)
@@ -93,6 +93,41 @@ impl Header {
     pub fn content_size(&self) -> u64 {
         self.full_size() - (size_of::<Self>() as u64)
     }
+
+    #[inline]
+    pub fn max_content_size(&self) -> u64 {
+        match self.htype {
+            // + null-termination
+            PXAR_FILENAME => crate::util::MAX_FILENAME_LEN + 1,
+            // + null-termination
+            PXAR_SYMLINK => crate::util::MAX_PATH_LEN + 1,
+            // + null-termination + offset
+            PXAR_HARDLINK => crate::util::MAX_PATH_LEN + 1 + (size_of::<u64>() as u64),
+            PXAR_DEVICE => size_of::<Device>() as u64,
+            PXAR_XATTR | PXAR_FCAPS => crate::util::MAX_XATTR_LEN,
+            PXAR_ACL_USER | PXAR_ACL_DEFAULT_USER => size_of::<acl::User>() as u64,
+            PXAR_ACL_GROUP | PXAR_ACL_DEFAULT_GROUP => size_of::<acl::Group>() as u64,
+            PXAR_ACL_DEFAULT => size_of::<acl::Default>() as u64,
+            PXAR_ACL_GROUP_OBJ => size_of::<acl::GroupObject> as u64,
+            PXAR_QUOTA_PROJID => size_of::<QuotaProjectId>() as u64,
+            PXAR_ENTRY => size_of::<Entry>() as u64,
+            PXAR_PAYLOAD | PXAR_GOODBYE => u64::MAX - (size_of::<Self>() as u64),
+            _ => u64::MAX - (size_of::<Self>() as u64),
+        }
+    }
+
+    #[inline]
+    pub fn check_header_size(&self) -> io::Result<()> {
+        if self.full_size() < size_of::<Header>() as u64 {
+            io_bail!("invalid header {} - too small ({})", self, self.full_size());
+        }
+
+        if self.content_size() > self.max_content_size() {
+            io_bail!("invalid content size ({} > {}) of entry with {}", self.content_size(), self.max_content_size(), self);
+        }
+        Ok(())
+    }
+}
 }
 
 #[derive(Clone, Debug, Default, Endian)]
index 5a3df62a07646ec440fc4bce78a34e6055662438..79a8785b96c97536fc25cf5ca65995bc23e7eb8d 100644 (file)
@@ -114,6 +114,12 @@ unsafe fn forbid_wake(_: *const ()) {
 
 unsafe fn ignore_drop(_: *const ()) {}
 
+pub const MAX_PATH_LEN:u64 = 4 * 1024;
+// let's play it safe
+pub const MAX_FILENAME_LEN:u64 = MAX_PATH_LEN;
+// name + attr
+pub const MAX_XATTR_LEN:u64 = 255 + 64*1024;
+
 pub fn validate_filename(name: &[u8]) -> io::Result<()> {
     if name.is_empty() {
         io_bail!("illegal path found (empty)");
@@ -131,5 +137,9 @@ pub fn validate_filename(name: &[u8]) -> io::Result<()> {
         io_bail!("illegal path found: '..'");
     }
 
+    if (name.len() as u64) > MAX_FILENAME_LEN {
+        io_bail!("filename too long (> {})", MAX_FILENAME_LEN);
+    }
+
     Ok(())
 }