Skip to content

Commit

Permalink
Auto merge of #11366 - ehuss:fix-safe-directory, r=epage
Browse files Browse the repository at this point in the history
Fix git2 safe-directory disable

The call to `set_verify_owner_validation` was not getting called unless a network configuration was found. This means in the common case that `cargo new` will fail when there is a safe-directory error. This fixes the issue by making sure that `set_verify_owner_validation` is called before the early-exits in `init_git_transports`.

Fixes #11365
  • Loading branch information
bors authored and ehuss committed Nov 15, 2022
1 parent 7e484fc commit 6774bc1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 24 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Run with 'cargo -Z [FLAG] [COMMAND]'",
}
};
config_configure(config, &expanded_args, subcommand_args, global_args)?;
super::init_git_transports(config);
super::init_git(config);

execute_subcommand(config, cmd, subcommand_args)
}
Expand Down
55 changes: 32 additions & 23 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,38 @@ fn search_directories(config: &Config) -> Vec<PathBuf> {
path_dirs
}

/// Initialize libgit2.
fn init_git(config: &Config) {
// Disabling the owner validation in git can, in theory, lead to code execution
// vulnerabilities. However, libgit2 does not launch executables, which is the foundation of
// the original security issue. Meanwhile, issues with refusing to load git repos in
// `CARGO_HOME` for example will likely be very frustrating for users. So, we disable the
// validation.
//
// For further discussion of Cargo's current interactions with git, see
//
// https://github.com/rust-lang/rfcs/pull/3279
//
// and in particular the subsection on "Git support".
//
// Note that we only disable this when Cargo is run as a binary. If Cargo is used as a library,
// this code won't be invoked. Instead, developers will need to explicitly disable the
// validation in their code. This is inconvenient, but won't accidentally open consuming
// applications up to security issues if they use git2 to open repositories elsewhere in their
// code.
unsafe {
git2::opts::set_verify_owner_validation(false)
.expect("set_verify_owner_validation should never fail");
}

init_git_transports(config);
}

/// Configure libgit2 to use libcurl if necessary.
///
/// If the user has a non-default network configuration, then libgit2 will be
/// configured to use libcurl instead of the built-in networking support so
/// that those configuration settings can be used.
fn init_git_transports(config: &Config) {
// Only use a custom transport if any HTTP options are specified,
// such as proxies or custom certificate authorities. The custom
Expand Down Expand Up @@ -274,27 +306,4 @@ fn init_git_transports(config: &Config) {
unsafe {
git2_curl::register(handle);
}

// Disabling the owner validation in git can, in theory, lead to code execution
// vulnerabilities. However, libgit2 does not launch executables, which is the foundation of
// the original security issue. Meanwhile, issues with refusing to load git repos in
// `CARGO_HOME` for example will likely be very frustrating for users. So, we disable the
// validation.
//
// For further discussion of Cargo's current interactions with git, see
//
// https://github.com/rust-lang/rfcs/pull/3279
//
// and in particular the subsection on "Git support".
//
// Note that we only disable this when Cargo is run as a binary. If Cargo is used as a library,
// this code won't be invoked. Instead, developers will need to explicitly disable the
// validation in their code. This is inconvenient, but won't accidentally open consuming
// applications up to security issues if they use git2 to open repositories elsewhere in their
// code.
unsafe {
if git2::opts::set_verify_owner_validation(false).is_err() {
return;
}
}
}

0 comments on commit 6774bc1

Please sign in to comment.