update: stage replacements exclusively and refuse oversized tar metadata (#563)

* update: stage replacements exclusively and refuse oversized tar metadata

- self_update: stage_copy creates the staged file beside the target with
  an unpredictable name (tempfile, O_EXCL) and writes bytes and mode
  through the handle, for both the AppImage and the tarball standalone
  replacement; a link at the old .<name>.update names is never followed.
- install::tree::unpack: check_metadata walks the tar headers first and
  refuses PAX or GNU long-name records over 1 MiB, which the tar crate
  buffers whole before the entry loop sees a size; both passes read at
  most MAX_TAR_BYTES of decompressed stream.

* address review: refuse size-truncated update archives, fixed staging prefix

crates/update/src/install.rs

@@ -1939,18 +1939,61 @@ pub mod macos {
 #[cfg(unix)]
 #[allow(dead_code)]
 pub(crate) mod tree {
+    use std::io::Read;
+
     use super::{App, Path, PathBuf, single_top_folder};
 
+    // Bound entry count and cumulative unpacked bytes to prevent decompression bombs (CWE-409).
+    const MAX_ENTRIES: usize = 50_000;
+    const MAX_UNPACKED_BYTES: u64 = 2 * 1024 * 1024 * 1024;
+    /// The most decompressed tar stream either pass reads: the unpacked bytes plus room for
+    /// headers, padding and metadata records.
+    const MAX_TAR_BYTES: u64 = MAX_UNPACKED_BYTES + (1 << 30);
+    /// The most a PAX or GNU long-name record may hold. The tar crate buffers such a record
+    /// whole while the iterator advances, before the loop below sees any entry size.
+    const MAX_METADATA_BYTES: u64 = 1 << 20;
+
+    /// Walks the headers of the tarball `file` without unpacking anything, and refuses a
+    /// metadata record (PAX `x`/`g`, GNU `L`/`K`) past [`MAX_METADATA_BYTES`]. A damaged
+    /// archive passes: unpacking reports it.
+    fn check_metadata(file: &Path) -> Result<(), String> {
+        let gz = std::fs::File::open(file).map_err(|e| e.to_string())?;
+        let mut tar = flate2::read::GzDecoder::new(gz).take(MAX_TAR_BYTES);
+        let mut header = [0u8; 512];
+        while tar.read_exact(&mut header).is_ok() && header.iter().any(|b| *b != 0) {
+            let size = header_size(&header[124..136]).ok_or("invalid tar header")?;
+            if matches!(header[156], b'x' | b'g' | b'L' | b'K') && size > MAX_METADATA_BYTES {
+                return Err("archive has an oversized metadata record".into());
+            }
+            let padded = size.div_ceil(512).saturating_mul(512);
+            if std::io::copy(&mut (&mut tar).take(padded), &mut std::io::sink()).map_err(|e| e.to_string())? < padded {
+                break;
+            }
+        }
+        Ok(())
+    }
+
+    /// A tar header's size field: octal digits, or big-endian base-256 after a set top bit.
+    fn header_size(field: &[u8]) -> Option<u64> {
+        if field.first()? & 0x80 != 0 {
+            return field
+                .iter()
+                .enumerate()
+                .try_fold(0u64, |n, (i, b)| n.checked_mul(256)?.checked_add(u64::from(if i == 0 { b & 0x7f } else { *b })));
+        }
+        let digits = std::str::from_utf8(field).ok()?.trim_matches(|c: char| c == '\0' || c == ' ');
+        if digits.is_empty() { Some(0) } else { u64::from_str_radix(digits, 8).ok() }
+    }
+
     /// Unpacks `file` and returns a staging folder laid out like `/usr/local`.
     pub fn unpack(app: &App, file: &Path, staging: &Path) -> Result<PathBuf, String> {
+        check_metadata(file)?;
         let _ = std::fs::remove_dir_all(staging);
         std::fs::create_dir_all(staging).map_err(|e| format!("couldn't create {} ({e})", staging.display()))?;
         let gz = std::fs::File::open(file).map_err(|e| e.to_string())?;
-        let mut archive = tar::Archive::new(flate2::read::GzDecoder::new(gz));
+        // Never past what `check_metadata` read.
+        let mut archive = tar::Archive::new(flate2::read::GzDecoder::new(gz).take(MAX_TAR_BYTES));
         archive.set_preserve_permissions(true);
-        // Bound entry count and cumulative unpacked bytes to prevent decompression bombs (CWE-409).
-        const MAX_ENTRIES: usize = 50_000;
-        const MAX_UNPACKED_BYTES: u64 = 2 * 1024 * 1024 * 1024;
         let mut entry_count = 0;
         let mut total_unpacked = 0u64;
         let mut directories = Vec::new();
@@ -1977,6 +2020,10 @@ pub(crate) mod tree {
         for mut dir in directories {
             dir.unpack_in(staging).map_err(|e| format!("couldn't unpack archive directory ({e})"))?;
         }
+        // tar reads a limit-truncated stream as a clean end of archive, so an exhausted limit is refused here.
+        if archive.into_inner().limit() == 0 {
+            return Err("archive exceeds maximum size".into());
+        }
         let top = single_top_folder(staging)?;
         if top.join("bin").join(app.id).is_file() {
             return Ok(top);
@@ -2739,6 +2786,26 @@ mod tests {
         assert!(tree::unpack(app, &bad, &dir.path().join("staging")).is_err());
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn an_oversized_pax_record_is_refused_before_tar_buffers_it() {
+        use std::io::Write;
+        let dir = tempfile::tempdir().unwrap();
+        // A PAX header claiming 1 MiB + 1 byte of records, compressed to almost nothing.
+        let mut header = tar::Header::new_ustar();
+        header.set_entry_type(tar::EntryType::XHeader);
+        header.set_size((1 << 20) + 1);
+        header.set_path("pax").unwrap();
+        header.set_cksum();
+        let file = dir.path().join("pax.tar.gz");
+        let mut gz = flate2::write::GzEncoder::new(std::fs::File::create(&file).unwrap(), flate2::Compression::best());
+        gz.write_all(header.as_bytes()).unwrap();
+        gz.write_all(&vec![b'a'; (1 << 20) + 512]).unwrap();
+        gz.finish().unwrap();
+        let err = tree::unpack(App::by_id("wordcraft").unwrap(), &file, &dir.path().join("staging")).unwrap_err();
+        assert!(err.contains("oversized metadata"), "{err}");
+    }
+
     #[cfg(unix)]
     #[test]
     fn tree_files_does_not_traverse_directory_symlinks() {

crates/update/src/self_update.rs

@@ -1068,12 +1068,9 @@ fn unmark_unless_scheduled<T>(paths: &Paths, scheduled: Result<T, String>) -> Re
 /// mounted from its own (now unlinked) file, so it keeps working until it exits.
 #[cfg(target_os = "linux")]
 fn replace_appimage(new: &Path, path: &Path) -> Result<(), String> {
-    use std::os::unix::fs::PermissionsExt;
-    let dir = path.parent().ok_or("the AppImage isn't in a folder")?;
-    let staged = dir.join(format!(".{}.update", path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default()));
-    std::fs::copy(new, &staged).map_err(|e| format!("couldn't write next to {} ({e})", path.display()))?;
+    path.parent().ok_or("the AppImage isn't in a folder")?;
     // The replacement AppImage must be executable; 0o755 is not group/world-writable.
-    std::fs::set_permissions(&staged, std::fs::Permissions::from_mode(0o755)).map_err(|e| e.to_string())?; // NOSONAR
+    let staged = stage_copy(new, path, 0o755)?; // NOSONAR
     std::fs::rename(&staged, path).map_err(|e| {
         let _ = std::fs::remove_file(&staged);
         format!("couldn't replace {} ({e})", path.display())
@@ -1185,20 +1182,24 @@ fn stage(from: &Path, to: &Path, executable: bool) -> Result<PathBuf, String> {
     use std::os::unix::fs::PermissionsExt;
     let dir = to.parent().ok_or_else(|| format!("{} isn't in a folder", to.display()))?;
     std::fs::create_dir_all(dir).map_err(|e| format!("couldn't create {} ({e})", dir.display()))?;
-    let name = to.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
-    let staged = dir.join(format!(".{name}.{}.update", std::process::id()));
-    let done = std::fs::copy(from, &staged).map_err(|e| format!("couldn't write next to {} ({e})", to.display())).and_then(|_| {
-        let mode = std::fs::metadata(from).map(|m| m.permissions().mode()).unwrap_or(0o644);
-        let mode = if executable { 0o755 } else { mode & 0o755 };
-        std::fs::set_permissions(&staged, std::fs::Permissions::from_mode(mode)).map_err(|e| e.to_string())
-    });
-    match done {
-        Ok(()) => Ok(staged),
-        Err(e) => {
-            let _ = std::fs::remove_file(&staged);
-            Err(e)
-        }
-    }
+    let mode = std::fs::metadata(from).map(|m| m.permissions().mode()).unwrap_or(0o644);
+    stage_copy(from, to, if executable { 0o755 } else { mode & 0o755 })
+}
+
+/// Copies `from` to a new file beside `to` and returns its path. The file is created exclusively
+/// under an unpredictable name, so a link someone left at a guessable name is never followed,
+/// and its bytes and `mode` go through the open handle. Removed again when any step fails.
+#[cfg(unix)]
+fn stage_copy(from: &Path, to: &Path, mode: u32) -> Result<PathBuf, String> {
+    use std::os::unix::fs::PermissionsExt;
+    let dir = to.parent().ok_or_else(|| format!("{} isn't in a folder", to.display()))?;
+    let write_error = |e: std::io::Error| format!("couldn't write next to {} ({e})", to.display());
+    // fixed prefix: the temp name's length must not grow with the destination's basename
+    let mut staged = tempfile::Builder::new().prefix(".staged-").suffix(".update").tempfile_in(dir).map_err(write_error)?;
+    let mut source = std::fs::File::open(from).map_err(|e| format!("couldn't read {} ({e})", from.display()))?;
+    std::io::copy(&mut source, staged.as_file_mut()).map_err(write_error)?;
+    staged.as_file().set_permissions(std::fs::Permissions::from_mode(mode)).map_err(|e| e.to_string())?;
+    staged.into_temp_path().keep().map_err(|e| e.to_string())
 }
 
 /// A regular file, not a link to one.
@@ -1730,6 +1731,22 @@ fn step_after_wait(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Fin
 mod tests {
     use super::*;
 
+    #[cfg(unix)]
+    #[test]
+    fn staging_never_writes_through_a_link_at_the_old_staging_name() {
+        let dir = tempfile::tempdir().unwrap();
+        let (new, exe, victim) = (dir.path().join("new"), dir.path().join("app"), dir.path().join("victim"));
+        std::fs::write(&new, b"replacement").unwrap();
+        std::fs::write(&exe, b"old").unwrap();
+        std::fs::write(&victim, b"keep me").unwrap();
+        // The names staging used to take: `.app.update` and `.app.<pid>.update`.
+        std::os::unix::fs::symlink(&victim, dir.path().join(".app.update")).unwrap();
+        std::os::unix::fs::symlink(&victim, dir.path().join(format!(".app.{}.update", std::process::id()))).unwrap();
+        let staged = stage_copy(&new, &exe, 0o755).unwrap();
+        assert_eq!(std::fs::read(&staged).unwrap(), b"replacement");
+        assert!(std::fs::symlink_metadata(&staged).unwrap().file_type().is_file());
+        assert_eq!(std::fs::read(&victim).unwrap(), b"keep me");
+    }
     #[test]
     fn macos_package_manager_folders_are_recognised() {
         for owned in [