[master] Fix for CVE-2022-36113 and CVE-2022-36114

crates/cargo-test-support/src/registry.rs

@@ -403,10 +403,17 @@ pub struct Dependency {
     optional: bool,
 }
 
+/// Entry with data that corresponds to [`tar::EntryType`].
+#[non_exhaustive]
+enum EntryData {
+    Regular(String),
+    Symlink(PathBuf),
+}
+
 /// A file to be created in a package.
 struct PackageFile {
     path: String,
-    contents: String,
+    contents: EntryData,
     /// The Unix mode for the file. Note that when extracted on Windows, this
     /// is mostly ignored since it doesn't have the same style of permissions.
     mode: u32,
@@ -780,13 +787,24 @@ impl Package {
     pub fn file_with_mode(&mut self, path: &str, mode: u32, contents: &str) -> &mut Package {
         self.files.push(PackageFile {
             path: path.to_string(),
-            contents: contents.to_string(),
+            contents: EntryData::Regular(contents.into()),
             mode,
             extra: false,
         });
         self
     }
 
+    /// Adds a symlink to a path to the package.
+    pub fn symlink(&mut self, dst: &str, src: &str) -> &mut Package {
+        self.files.push(PackageFile {
+            path: dst.to_string(),
+            contents: EntryData::Symlink(src.into()),
+            mode: DEFAULT_MODE,
+            extra: false,
+        });
+        self
+    }
+
     /// Adds an "extra" file that is not rooted within the package.
     ///
     /// Normal files are automatically placed within a directory named
@@ -795,7 +813,7 @@ impl Package {
     pub fn extra_file(&mut self, path: &str, contents: &str) -> &mut Package {
         self.files.push(PackageFile {
             path: path.to_string(),
-            contents: contents.to_string(),
+            contents: EntryData::Regular(contents.to_string()),
             mode: DEFAULT_MODE,
             extra: true,
         });
@@ -1033,7 +1051,12 @@ impl Package {
             self.append_manifest(&mut a);
         }
         if self.files.is_empty() {
-            self.append(&mut a, "src/lib.rs", DEFAULT_MODE, "");
+            self.append(
+                &mut a,
+                "src/lib.rs",
+                DEFAULT_MODE,
+                &EntryData::Regular("".into()),
+            );
         } else {
             for PackageFile {
                 path,
@@ -1107,10 +1130,15 @@ impl Package {
             manifest.push_str("[lib]\nproc-macro = true\n");
         }
 
-        self.append(ar, "Cargo.toml", DEFAULT_MODE, &manifest);
+        self.append(
+            ar,
+            "Cargo.toml",
+            DEFAULT_MODE,
+            &EntryData::Regular(manifest.into()),
+        );
     }
 
-    fn append<W: Write>(&self, ar: &mut Builder<W>, file: &str, mode: u32, contents: &str) {
+    fn append<W: Write>(&self, ar: &mut Builder<W>, file: &str, mode: u32, contents: &EntryData) {
         self.append_raw(
             ar,
             &format!("{}-{}/{}", self.name, self.vers, file),
@@ -1119,8 +1147,22 @@ impl Package {
         );
     }
 
-    fn append_raw<W: Write>(&self, ar: &mut Builder<W>, path: &str, mode: u32, contents: &str) {
+    fn append_raw<W: Write>(
+        &self,
+        ar: &mut Builder<W>,
+        path: &str,
+        mode: u32,
+        contents: &EntryData,
+    ) {
         let mut header = Header::new_ustar();
+        let contents = match contents {
+            EntryData::Regular(contents) => contents.as_str(),
+            EntryData::Symlink(src) => {
+                header.set_entry_type(tar::EntryType::Symlink);
+                t!(header.set_link_name(src));
+                "" // Symlink has no contents.
+            }
+        };
         header.set_size(contents.len() as u64);
         t!(header.set_path(path));
         header.set_mode(mode);

src/cargo/sources/registry/mod.rs

@@ -182,7 +182,9 @@ use crate::util::hex;
 use crate::util::interning::InternedString;
 use crate::util::into_url::IntoUrl;
 use crate::util::network::PollExt;
-use crate::util::{restricted_names, CargoResult, Config, Filesystem, OptVersionReq};
+use crate::util::{
+    restricted_names, CargoResult, Config, Filesystem, LimitErrorReader, OptVersionReq,
+};
 
 const PACKAGE_SOURCE_LOCK: &str = ".cargo-ok";
 pub const CRATES_IO_INDEX: &str = "/~https://github.com/rust-lang/crates.io-index";
@@ -194,6 +196,7 @@ const VERSION_TEMPLATE: &str = "{version}";
 const PREFIX_TEMPLATE: &str = "{prefix}";
 const LOWER_PREFIX_TEMPLATE: &str = "{lowerprefix}";
 const CHECKSUM_TEMPLATE: &str = "{sha256-checksum}";
+const MAX_UNPACK_SIZE: u64 = 512 * 1024 * 1024;
 
 /// A "source" for a local (see `local::LocalRegistry`) or remote (see
 /// `remote::RemoteRegistry`) registry.
@@ -615,6 +618,7 @@ impl<'cfg> RegistrySource<'cfg> {
             }
         }
         let gz = GzDecoder::new(tarball);
+        let gz = LimitErrorReader::new(gz, max_unpack_size());
         let mut tar = Archive::new(gz);
         let prefix = unpack_dir.file_name().unwrap();
         let parent = unpack_dir.parent().unwrap();
@@ -639,6 +643,13 @@ impl<'cfg> RegistrySource<'cfg> {
                     prefix
                 )
             }
+            // Prevent unpacking the lockfile from the crate itself.
+            if entry_path
+                .file_name()
+                .map_or(false, |p| p == PACKAGE_SOURCE_LOCK)
+            {
+                continue;
+            }
             // Unpacking failed
             let mut result = entry.unpack_in(parent).map_err(anyhow::Error::from);
             if cfg!(windows) && restricted_names::is_windows_reserved_path(&entry_path) {
@@ -654,16 +665,14 @@ impl<'cfg> RegistrySource<'cfg> {
                 .with_context(|| format!("failed to unpack entry at `{}`", entry_path.display()))?;
         }
 
-        // The lock file is created after unpacking so we overwrite a lock file
-        // which may have been extracted from the package.
+        // Now that we've finished unpacking, create and write to the lock file to indicate that
+        // unpacking was successful.
         let mut ok = OpenOptions::new()
-            .create(true)
+            .create_new(true)
             .read(true)
             .write(true)
             .open(&path)
             .with_context(|| format!("failed to open `{}`", path.display()))?;
-
-        // Write to the lock file to indicate that unpacking was successful.
         write!(ok, "ok")?;
 
         Ok(unpack_dir.to_path_buf())
@@ -826,6 +835,20 @@ impl<'cfg> Source for RegistrySource<'cfg> {
     }
 }
 
+/// For integration test only.
+#[inline]
+fn max_unpack_size() -> u64 {
+    const VAR: &str = "__CARGO_TEST_MAX_UNPACK_SIZE";
+    if cfg!(debug_assertions) && std::env::var(VAR).is_ok() {
+        std::env::var(VAR)
+            .unwrap()
+            .parse()
+            .expect("a max unpack size in bytes")
+    } else {
+        MAX_UNPACK_SIZE
+    }
+}
+
 fn make_dep_prefix(name: &str) -> String {
     match name.len() {
         1 => String::from("1"),

src/cargo/util/io.rs

@@ -0,0 +1,51 @@
+use std::io::{self, Read, Take};
+
+#[derive(Debug)]
+pub struct LimitErrorReader<R> {
+    inner: Take<R>,
+}
+
+impl<R: Read> LimitErrorReader<R> {
+    pub fn new(r: R, limit: u64) -> LimitErrorReader<R> {
+        LimitErrorReader {
+            inner: r.take(limit),
+        }
+    }
+}
+
+impl<R: Read> Read for LimitErrorReader<R> {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        match self.inner.read(buf) {
+            Ok(0) if self.inner.limit() == 0 => Err(io::Error::new(
+                io::ErrorKind::Other,
+                "maximum limit reached when reading",
+            )),
+            e => e,
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::LimitErrorReader;
+
+    use std::io::Read;
+
+    #[test]
+    fn under_the_limit() {
+        let buf = &[1; 7][..];
+        let mut r = LimitErrorReader::new(buf, 8);
+        let mut out = Vec::new();
+        assert!(matches!(r.read_to_end(&mut out), Ok(7)));
+        assert_eq!(buf, out.as_slice());
+    }
+
+    #[test]
+    #[should_panic = "maximum limit reached when reading"]
+    fn over_the_limit() {
+        let buf = &[1; 8][..];
+        let mut r = LimitErrorReader::new(buf, 8);
+        let mut out = Vec::new();
+        r.read_to_end(&mut out).unwrap();
+    }
+}

src/cargo/util/mod.rs

@@ -14,6 +14,7 @@ pub use self::hasher::StableHasher;
 pub use self::hex::{hash_u64, short_hash, to_hex};
 pub use self::into_url::IntoUrl;
 pub use self::into_url_with_base::IntoUrlWithBase;
+pub(crate) use self::io::LimitErrorReader;
 pub use self::lev_distance::{closest, closest_msg, lev_distance};
 pub use self::lockserver::{LockServer, LockServerClient, LockServerStarted};
 pub use self::progress::{Progress, ProgressStyle};
@@ -44,6 +45,7 @@ pub mod important_paths;
 pub mod interning;
 pub mod into_url;
 mod into_url_with_base;
+mod io;
 pub mod job;
 pub mod lev_distance;
 mod lockserver;

tests/testsuite/registry.rs

@@ -2583,6 +2583,47 @@ fn package_lock_inside_package_is_overwritten() {
     assert_eq!(ok.metadata().unwrap().len(), 2);
 }
 
+#[cargo_test]
+fn package_lock_as_a_symlink_inside_package_is_overwritten() {
+    let registry = registry::init();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+
+                [dependencies]
+                bar = ">= 0.0.0"
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1")
+        .file("src/lib.rs", "pub fn f() {}")
+        .symlink(".cargo-ok", "src/lib.rs")
+        .publish();
+
+    p.cargo("build").run();
+
+    let id = SourceId::for_registry(registry.index_url()).unwrap();
+    let hash = cargo::util::hex::short_hash(&id);
+    let pkg_root = cargo_home()
+        .join("registry")
+        .join("src")
+        .join(format!("-{}", hash))
+        .join("bar-0.0.1");
+    let ok = pkg_root.join(".cargo-ok");
+    let librs = pkg_root.join("src/lib.rs");
+
+    // Is correctly overwritten and doesn't affect the file linked to
+    assert_eq!(ok.metadata().unwrap().len(), 2);
+    assert_eq!(fs::read_to_string(librs).unwrap(), "pub fn f() {}");
+}
+
 #[cargo_test]
 fn ignores_unknown_index_version_http() {
     let _server = setup_http();
@@ -2656,3 +2697,45 @@ fn http_requires_trailing_slash() {
         .with_stderr("[ERROR] registry url must end in a slash `/`: sparse+https://index.crates.io")
         .run()
 }
+
+#[cargo_test]
+fn reach_max_unpack_size() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+
+                [dependencies]
+                bar = ">= 0.0.0"
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1").publish();
+
+    p.cargo("build")
+        .env("__CARGO_TEST_MAX_UNPACK_SIZE", "8") // hit 8 bytes limit and boom!
+        .with_status(101)
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
+[ERROR] failed to download replaced source registry `crates-io`
+
+Caused by:
+  failed to unpack package `bar v0.0.1 (registry `dummy-registry`)`
+
+Caused by:
+  failed to iterate over archive
+
+Caused by:
+  maximum limit reached when reading
+",
+        )
+        .run();
+}