Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,13 @@ mod test {
assert_eq!(ident1, ident2);
}

#[test]
fn test_canonicalize_idents_does_not_strip_dot_git_for_sparse() {
let ident1 = ident(&src("sparse+https://crates.io/fake-registry"));
let ident2 = ident(&src("sparse+https://crates.io/fake-registry.git"));
assert_ne!(ident1, ident2);
}

fn src(s: &str) -> SourceId {
SourceId::for_git(&s.into_url().unwrap(), GitReference::DefaultBranch).unwrap()
}
Expand Down
10 changes: 9 additions & 1 deletion src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ use flate2::read::GzDecoder;
use futures::FutureExt as _;
use serde::Deserialize;
use serde::Serialize;
use tar::Archive;
use tar::{Archive, EntryType};
use tracing::debug;

use crate::core::dependency::Dependency;
Expand Down Expand Up @@ -1019,6 +1019,14 @@ fn unpack(
)
}

// Prevent unpacking symlinks and other unexpected entry types
match entry.header().entry_type() {
EntryType::Regular | EntryType::Directory => {}
t => anyhow::bail!(
"invalid tarball downloaded, contains an entry at {entry_path:?} with invalid type {t:?}",
),
}

// Prevent unpacking the lockfile from the crate itself.
if entry_path
.file_name()
Expand Down
44 changes: 24 additions & 20 deletions src/cargo/util/canonical_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,31 @@ impl CanonicalUrl {
url.path_segments_mut().unwrap().pop_if_empty();
}

// For GitHub URLs specifically, just lower-case everything. GitHub
// treats both the same, but they hash differently, and we're gonna be
// hashing them. This wants a more general solution, and also we're
// almost certainly not using the same case conversion rules that GitHub
// does. (See issue #84)
if url.host_str() == Some("github.com") {
url = format!("https{}", &url[url::Position::AfterScheme..])
.parse()
.unwrap();
let path = url.path().to_lowercase();
url.set_path(&path);
}
// Perform further canonicalization specific to git registries, which
// do not contain a `+` specifier.
if !url.scheme().contains('+') {
// For GitHub URLs specifically, just lower-case everything. GitHub
// treats both the same, but they hash differently, and we're gonna be
// hashing them. This wants a more general solution, and also we're
// almost certainly not using the same case conversion rules that GitHub
// does. (See issue #84)
if url.host_str() == Some("github.com") {
url = format!("https{}", &url[url::Position::AfterScheme..])
.parse()
.unwrap();
let path = url.path().to_lowercase();
url.set_path(&path);
}

// Repos can generally be accessed with or without `.git` extension.
let needs_chopping = url.path().ends_with(".git");
if needs_chopping {
let last = {
let last = url.path_segments().unwrap().next_back().unwrap();
last[..last.len() - 4].to_owned()
};
url.path_segments_mut().unwrap().pop().push(&last);
// Repos can generally be accessed with or without `.git` extension.
let needs_chopping = url.path().ends_with(".git");
if needs_chopping {
let last = {
let last = url.path_segments().unwrap().next_back().unwrap();
last[..last.len() - 4].to_owned()
};
url.path_segments_mut().unwrap().pop().push(&last);
}
}

Ok(CanonicalUrl(url))
Expand Down
39 changes: 17 additions & 22 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3276,8 +3276,7 @@ fn package_lock_inside_package_is_overwritten() {
}

#[cargo_test]
fn package_lock_as_a_symlink_inside_package_is_overwritten() {
let registry = registry::init();
fn package_lock_as_a_symlink_inside_package_is_invalid() {
let p = project()
.file(
"Cargo.toml",
Expand All @@ -3300,21 +3299,23 @@ fn package_lock_as_a_symlink_inside_package_is_overwritten() {
.symlink(".cargo-ok", "src/lib.rs")
.publish();

p.cargo("check").run();
p.cargo("check")
.with_status(101)
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 1 package to latest compatible version
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
[ERROR] failed to download replaced source registry `crates-io`

let id = SourceId::for_registry(registry.index_url()).unwrap();
let hash = cargo::util::hex::short_hash(&id);
let pkg_root = paths::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");
Caused by:
failed to unpack package `bar v0.0.1 (registry `dummy-registry`)`

// Is correctly overwritten and doesn't affect the file linked to
assert_eq!(ok.metadata().unwrap().len(), 7);
assert_eq!(fs::read_to_string(librs).unwrap(), "pub fn f() {}");
Caused by:
invalid tarball downloaded, contains an entry at "bar-0.0.1/.cargo-ok" with invalid type Symlink

"#]])
.run();
}

#[cargo_test]
Expand Down Expand Up @@ -4770,13 +4771,7 @@ Caused by:
failed to unpack package `bar v1.0.0 (registry `dummy-registry`)`

Caused by:
failed to unpack entry at `bar-1.0.0/smuggled`

Caused by:
failed to unpack `[ROOT]/home/.cargo/registry/src/-[HASH]/bar-1.0.0/smuggled`

Caused by:
[..] when creating dir [ROOT]/home/.cargo/registry/src/-[HASH]/bar-1.0.0/smuggled
invalid tarball downloaded, contains an entry at "bar-1.0.0/smuggled" with invalid type Symlink

"#]])
.run();
Expand Down
Loading