fix(verifier): confine image archive extraction

dstack/verifier/Cargo.toml

@@ -34,6 +34,8 @@ tracing.workspace = true
 tracing-subscriber = { workspace = true, optional = true }
 reqwest.workspace = true
 tempfile.workspace = true
+flate2.workspace = true
+tar.workspace = true
 
 # Internal dependencies
 ra-tls.workspace = true

dstack/verifier/src/verification.rs

@@ -4,7 +4,7 @@
 
 use std::{
     ffi::OsStr,
-    path::{Path, PathBuf},
+    path::{Component, Path, PathBuf},
     sync::Arc,
     time::Duration,
 };
@@ -415,6 +415,90 @@ impl CvmVerifier {
             .is_some_and(|digest| digest == expected))
     }
 
+    /// Mirrors the confinement rule `tar::Entry::unpack_in` applies internally:
+    /// `..` components, absolute paths, and Windows prefixes escape the
+    /// extraction root, while `.` components are stripped and are harmless.
+    ///
+    /// This duplicates the library check on purpose. `Archive::unpack` discards
+    /// the `unpack_in` return value, so an escaping member is silently dropped
+    /// and extraction still reports success; checking here turns that into an
+    /// error and keeps the boundary from widening if the library's behavior
+    /// ever changes. It must not be *stricter* than the library, though:
+    /// rejecting `.` components would reject the `./`-prefixed archives that
+    /// `tar -czf out.tar.gz .` produces, and roughly a third of the images
+    /// published on download.dstack.org are packed that way.
+    fn is_confined_archive_path(path: &Path) -> bool {
+        path.components()
+            .all(|component| matches!(component, Component::Normal(_) | Component::CurDir))
+    }
+
+    /// A manifest name must be literally a file name, because
+    /// `prune_unlisted_image_files` matches manifest entries against the
+    /// `file_name()` of each top-level directory entry, and `sha256sum -c`
+    /// resolves them relative to the extraction root.
+    fn is_flat_manifest_name(name: &str) -> bool {
+        Path::new(name)
+            .file_name()
+            .is_some_and(|file_name| file_name == OsStr::new(name))
+    }
+
+    fn validate_image_manifest_paths(files_doc: &str) -> Result<()> {
+        for (line_index, line) in files_doc.lines().enumerate() {
+            if line.trim().is_empty() {
+                continue;
+            }
+            let mut fields = line.split_whitespace();
+            let _digest = fields
+                .next()
+                .context("image manifest entry is missing a digest")?;
+            let name = fields
+                .next()
+                .context("image manifest entry is missing a path")?;
+            if fields.next().is_some() {
+                bail!("image manifest line {} has extra fields", line_index + 1);
+            }
+            if !Self::is_flat_manifest_name(name) {
+                bail!("image manifest line {} has an unsafe path", line_index + 1);
+            }
+            if name == "sha256sum.txt" {
+                bail!("image manifest must not recursively list sha256sum.txt");
+            }
+        }
+        Ok(())
+    }
+
+    fn extract_image_archive(tarball_path: &Path, extracted_dir: &Path) -> Result<()> {
+        let file = fs_err::File::open(tarball_path).context("Failed to open image archive")?;
+        // `MultiGzDecoder`, not `GzDecoder`: the latter stops at the first gzip
+        // member and reports clean EOF, so a concatenated-member archive would
+        // extract partially without any error.
+        let decoder = flate2::read::MultiGzDecoder::new(file);
+        let mut archive = tar::Archive::new(decoder);
+        for entry in archive.entries().context("Failed to read image archive")? {
+            let mut entry = entry.context("Failed to read image archive entry")?;
+            let path = entry
+                .path()
+                .context("Failed to decode image archive path")?;
+            if !Self::is_confined_archive_path(&path) {
+                bail!("image archive contains unsafe path {}", path.display());
+            }
+            let entry_type = entry.header().entry_type();
+            if !(entry_type.is_file() || entry_type.is_dir()) {
+                bail!(
+                    "image archive contains unsupported entry {}",
+                    path.display()
+                );
+            }
+            if !entry
+                .unpack_in(extracted_dir)
+                .context("Failed to extract image archive entry")?
+            {
+                bail!("image archive entry escaped the extraction root");
+            }
+        }
+        Ok(())
+    }
+
     fn prune_unlisted_image_files(extracted_dir: &Path, files_doc: &str) -> Result<()> {
         let listed_files: Vec<&OsStr> = files_doc
             .lines()
@@ -1163,21 +1247,16 @@ impl CvmVerifier {
         let extracted_dir = tmp_dir.join("extracted");
         fs_err::create_dir_all(&extracted_dir).context("Failed to create extraction directory")?;
 
-        // Extract the tarball
-        let output = Command::new("tar")
-            .arg("xzf")
-            .arg(&tarball_path)
-            .current_dir(&extracted_dir)
-            .output()
+        file.flush()
             .await
-            .context("Failed to extract tarball")?;
+            .context("Failed to flush image archive")?;
+        drop(file);
+        Self::extract_image_archive(&tarball_path, &extracted_dir)?;
 
-        if !output.status.success() {
-            bail!(
-                "Failed to extract tarball: {}",
-                String::from_utf8_lossy(&output.stderr)
-            );
-        }
+        let sha256sum_path = extracted_dir.join("sha256sum.txt");
+        let files_doc =
+            fs_err::read_to_string(&sha256sum_path).context("Failed to read sha256sum.txt")?;
+        Self::validate_image_manifest_paths(&files_doc)?;
 
         // Verify checksum
         let output = Command::new("sha256sum")
@@ -1196,9 +1275,6 @@ impl CvmVerifier {
         }
 
         // Remove the files that are not listed in sha256sum.txt
-        let sha256sum_path = extracted_dir.join("sha256sum.txt");
-        let files_doc =
-            fs_err::read_to_string(&sha256sum_path).context("Failed to read sha256sum.txt")?;
         Self::prune_unlisted_image_files(&extracted_dir, &files_doc)?;
 
         // All image modes are addressed by sha256(sha256sum.txt). Extra
@@ -1414,6 +1490,164 @@ mod tests {
         assert!(!dir.path().join("unmeasured").exists());
     }
 
+    #[test]
+    fn image_paths_must_be_confined_and_manifest_paths_must_be_flat() {
+        for path in ["../escape", "/absolute", "nested/../escape"] {
+            assert!(
+                !CvmVerifier::is_confined_archive_path(Path::new(path)),
+                "{path}"
+            );
+        }
+        // `.` components are stripped by `unpack_in` and cannot escape, so the
+        // check must accept them: `tar -czf out.tar.gz .` prefixes every member
+        // with `./` and published images are packed that way.
+        for path in ["nested/artifact", "./metadata.json", ".", "./", ""] {
+            assert!(
+                CvmVerifier::is_confined_archive_path(Path::new(path)),
+                "{path}"
+            );
+        }
+
+        let digest = "00".repeat(32);
+        assert!(
+            CvmVerifier::validate_image_manifest_paths(&format!("{digest}  metadata.json\n"))
+                .is_ok()
+        );
+        for path in [
+            "../escape",
+            "/absolute",
+            "nested/artifact",
+            "./metadata.json",
+            ".",
+            "sha256sum.txt",
+        ] {
+            assert!(
+                CvmVerifier::validate_image_manifest_paths(&format!("{digest}  {path}\n")).is_err(),
+                "{path}"
+            );
+        }
+    }
+
+    #[test]
+    fn image_archive_rejects_links_and_accepts_regular_files() {
+        let directory = tempfile::tempdir().unwrap();
+        let valid = directory.path().join("valid.tar.gz");
+        {
+            let file = fs_err::File::create(&valid).unwrap();
+            let encoder = flate2::write::GzEncoder::new(file, flate2::Compression::default());
+            let mut archive = tar::Builder::new(encoder);
+            let payload = b"artifact";
+            let mut header = tar::Header::new_gnu();
+            header.set_size(payload.len() as u64);
+            header.set_mode(0o644);
+            header.set_cksum();
+            archive
+                .append_data(&mut header, "nested/artifact", &payload[..])
+                .unwrap();
+            archive.finish().unwrap();
+        }
+        let output = directory.path().join("valid-output");
+        fs_err::create_dir(&output).unwrap();
+        CvmVerifier::extract_image_archive(&valid, &output).unwrap();
+        assert_eq!(
+            fs_err::read(output.join("nested/artifact")).unwrap(),
+            b"artifact"
+        );
+
+        let linked = directory.path().join("linked.tar.gz");
+        {
+            let file = fs_err::File::create(&linked).unwrap();
+            let encoder = flate2::write::GzEncoder::new(file, flate2::Compression::default());
+            let mut archive = tar::Builder::new(encoder);
+            let mut header = tar::Header::new_gnu();
+            header.set_entry_type(tar::EntryType::Symlink);
+            header.set_size(0);
+            header.set_mode(0o777);
+            header.set_link_name("../outside").unwrap();
+            header.set_cksum();
+            archive.append_data(&mut header, "link", &[][..]).unwrap();
+            archive.finish().unwrap();
+        }
+        let output = directory.path().join("linked-output");
+        fs_err::create_dir(&output).unwrap();
+        assert!(CvmVerifier::extract_image_archive(&linked, &output).is_err());
+    }
+
+    /// Images published on download.dstack.org come in two shapes: members
+    /// packed from a glob (`bzImage`, ...) and members packed from `.`
+    /// (`./`, `./bzImage`, ...). Both must extract to the same flat layout.
+    #[test]
+    fn image_archive_accepts_dot_prefixed_members() {
+        let directory = tempfile::tempdir().unwrap();
+        let archive_path = directory.path().join("dot-prefixed.tar.gz");
+        {
+            let file = fs_err::File::create(&archive_path).unwrap();
+            let encoder = flate2::write::GzEncoder::new(file, flate2::Compression::default());
+            let mut archive = tar::Builder::new(encoder);
+            let mut header = tar::Header::new_gnu();
+            header.set_entry_type(tar::EntryType::Directory);
+            header.set_size(0);
+            header.set_mode(0o755);
+            header.set_cksum();
+            archive.append_data(&mut header, "./", &[][..]).unwrap();
+            let payload = b"artifact";
+            let mut header = tar::Header::new_gnu();
+            header.set_size(payload.len() as u64);
+            header.set_mode(0o644);
+            header.set_cksum();
+            archive
+                .append_data(&mut header, "./metadata.json", &payload[..])
+                .unwrap();
+            archive.finish().unwrap();
+        }
+        let output = directory.path().join("dot-prefixed-output");
+        fs_err::create_dir(&output).unwrap();
+        CvmVerifier::extract_image_archive(&archive_path, &output).unwrap();
+        assert_eq!(
+            fs_err::read(output.join("metadata.json")).unwrap(),
+            b"artifact"
+        );
+    }
+
+    /// `GzDecoder` stops at the first member of a concatenated gzip stream and
+    /// reports clean EOF, which would truncate the archive without an error.
+    #[test]
+    fn image_archive_reads_every_gzip_member() {
+        let directory = tempfile::tempdir().unwrap();
+        let mut tarball = tar::Builder::new(Vec::new());
+        let payload = b"artifact";
+        let mut header = tar::Header::new_gnu();
+        header.set_size(payload.len() as u64);
+        header.set_mode(0o644);
+        header.set_cksum();
+        tarball
+            .append_data(&mut header, "metadata.json", &payload[..])
+            .unwrap();
+        let tarball = tarball.into_inner().unwrap();
+
+        let archive_path = directory.path().join("multi-member.tar.gz");
+        {
+            use std::io::Write;
+
+            let mut file = fs_err::File::create(&archive_path).unwrap();
+            // One gzip member per half of the tar stream.
+            for half in tarball.chunks(tarball.len().div_ceil(2)) {
+                let mut encoder =
+                    flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default());
+                encoder.write_all(half).unwrap();
+                file.write_all(&encoder.finish().unwrap()).unwrap();
+            }
+            file.flush().unwrap();
+        }
+        let output = directory.path().join("multi-member-output");
+        fs_err::create_dir(&output).unwrap();
+        CvmVerifier::extract_image_archive(&archive_path, &output).unwrap();
+        assert_eq!(
+            fs_err::read(output.join("metadata.json")).unwrap(),
+            b"artifact"
+        );
+    }
+
     #[tokio::test]
     async fn verifies_sev_snp_attestation_fixture_without_image_download() {
         let request: VerificationRequest =