Compare commits
No commits in common. "eae0f55856392f4cdd099f74a2a968dafaf8b4ed" and "7fd96459c2e66e6ed72c26cf540e12ffae28ac9a" have entirely different histories.
eae0f55856
...
7fd96459c2
@ -1,213 +0,0 @@
|
||||
From d2da776547575648d6f9da4ac7b4b1d06d6ab1a8 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
|
||||
<avarab@gmail.com>
|
||||
Date: Thu, 1 Sep 2022 01:17:56 +0200
|
||||
Subject: [PATCH 1/3] submodule--helper: add "const" to passed
|
||||
"module_clone_data"
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Add "const" to the "struct module_clone_data" that we pass to
|
||||
clone_submodule(), which makes the ownership clear, and stops us from
|
||||
clobbering the "clone_data->path".
|
||||
|
||||
We still need to add to the "reference" member, which is a "struct
|
||||
string_list". Let's do this by having clone_submodule() create its
|
||||
own, and copy the contents over, allowing us to pass it as a
|
||||
separate parameter.
|
||||
|
||||
This new "struct string_list" still leaks memory, just as the "struct
|
||||
module_clone_data" did before. let's not fix that for now, to fix that
|
||||
we'll need to add some "goto cleanup" to the relevant code. That will
|
||||
eventually be done in follow-up commits, this change makes it easier
|
||||
to fix the memory leak.
|
||||
|
||||
The scope of the new "reference" variable in add_submodule() could be
|
||||
narrowed to the "else" block, but as we'll eventually free it with a
|
||||
"goto cleanup" let's declare it at the start of the function.
|
||||
|
||||
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
|
||||
Reviewed-by: Glen Choo <chooglen@google.com>
|
||||
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||||
Reference: https://git.kernel.org/pub/scm/git/git.git/commit/?id=6fac5b2f352efc8c246d6d5be63a66b7b0fc0209
|
||||
Conflicts:
|
||||
builtin/submodule--helper.c
|
||||
Signed-off-by: qiaojijun <qiaojijun@kylinos.cn>
|
||||
---
|
||||
builtin/submodule--helper.c | 49 ++++++++++++++++++++-----------------
|
||||
1 file changed, 27 insertions(+), 22 deletions(-)
|
||||
|
||||
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
|
||||
index ef2776a..7216e71 100644
|
||||
--- a/builtin/submodule--helper.c
|
||||
+++ b/builtin/submodule--helper.c
|
||||
@@ -1665,14 +1665,13 @@ struct module_clone_data {
|
||||
const char *name;
|
||||
const char *url;
|
||||
const char *depth;
|
||||
- struct string_list reference;
|
||||
unsigned int quiet: 1;
|
||||
unsigned int progress: 1;
|
||||
unsigned int dissociate: 1;
|
||||
unsigned int require_init: 1;
|
||||
int single_branch;
|
||||
};
|
||||
-#define MODULE_CLONE_DATA_INIT { .reference = STRING_LIST_INIT_NODUP, .single_branch = -1 }
|
||||
+#define MODULE_CLONE_DATA_INIT { .single_branch = -1 }
|
||||
|
||||
struct submodule_alternate_setup {
|
||||
const char *submodule_name;
|
||||
@@ -1778,12 +1777,14 @@ static void prepare_possible_alternates(const char *sm_name,
|
||||
free(error_strategy);
|
||||
}
|
||||
|
||||
-static int clone_submodule(struct module_clone_data *clone_data)
|
||||
+static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
+ struct string_list *reference)
|
||||
{
|
||||
char *p, *sm_gitdir;
|
||||
char *sm_alternate = NULL, *error_strategy = NULL;
|
||||
struct strbuf sb = STRBUF_INIT;
|
||||
struct child_process cp = CHILD_PROCESS_INIT;
|
||||
+ const char *clone_data_path;
|
||||
|
||||
strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), clone_data->name);
|
||||
sm_gitdir = absolute_pathdup(sb.buf);
|
||||
@@ -1791,9 +1792,10 @@ static int clone_submodule(struct module_clone_data *clone_data)
|
||||
|
||||
if (!is_absolute_path(clone_data->path)) {
|
||||
strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
|
||||
- clone_data->path = strbuf_detach(&sb, NULL);
|
||||
+ clone_data_path = xstrfmt("%s/%s", get_git_work_tree(),
|
||||
+ clone_data->path);
|
||||
} else {
|
||||
- clone_data->path = xstrdup(clone_data->path);
|
||||
+ clone_data_path = xstrdup(clone_data->path);
|
||||
}
|
||||
|
||||
if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
|
||||
@@ -1804,7 +1806,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
|
||||
if (safe_create_leading_directories_const(sm_gitdir) < 0)
|
||||
die(_("could not create directory '%s'"), sm_gitdir);
|
||||
|
||||
- prepare_possible_alternates(clone_data->name, &clone_data->reference);
|
||||
+ prepare_possible_alternates(clone_data->name, reference);
|
||||
|
||||
strvec_push(&cp.args, "clone");
|
||||
strvec_push(&cp.args, "--no-checkout");
|
||||
@@ -1814,9 +1816,9 @@ static int clone_submodule(struct module_clone_data *clone_data)
|
||||
strvec_push(&cp.args, "--progress");
|
||||
if (clone_data->depth && *(clone_data->depth))
|
||||
strvec_pushl(&cp.args, "--depth", clone_data->depth, NULL);
|
||||
- if (clone_data->reference.nr) {
|
||||
+ if (reference->nr) {
|
||||
struct string_list_item *item;
|
||||
- for_each_string_list_item(item, &clone_data->reference)
|
||||
+ for_each_string_list_item(item, reference)
|
||||
strvec_pushl(&cp.args, "--reference",
|
||||
item->string, NULL);
|
||||
}
|
||||
@@ -1831,7 +1833,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
|
||||
|
||||
strvec_push(&cp.args, "--");
|
||||
strvec_push(&cp.args, clone_data->url);
|
||||
- strvec_push(&cp.args, clone_data->path);
|
||||
+ strvec_push(&cp.args, clone_data_path);
|
||||
|
||||
cp.git_cmd = 1;
|
||||
prepare_submodule_repo_env(&cp.env_array);
|
||||
@@ -1839,23 +1841,23 @@ static int clone_submodule(struct module_clone_data *clone_data)
|
||||
|
||||
if(run_command(&cp))
|
||||
die(_("clone of '%s' into submodule path '%s' failed"),
|
||||
- clone_data->url, clone_data->path);
|
||||
+ clone_data->url, clone_data_path);
|
||||
} else {
|
||||
- if (clone_data->require_init && !access(clone_data->path, X_OK) &&
|
||||
- !is_empty_dir(clone_data->path))
|
||||
- die(_("directory not empty: '%s'"), clone_data->path);
|
||||
- if (safe_create_leading_directories_const(clone_data->path) < 0)
|
||||
- die(_("could not create directory '%s'"), clone_data->path);
|
||||
+ if (clone_data->require_init && !access(clone_data_path, X_OK) &&
|
||||
+ !is_empty_dir(clone_data_path))
|
||||
+ die(_("directory not empty: '%s'"), clone_data_path);
|
||||
+ if (safe_create_leading_directories_const(clone_data_path) < 0)
|
||||
+ die(_("could not create directory '%s'"), clone_data_path);
|
||||
strbuf_addf(&sb, "%s/index", sm_gitdir);
|
||||
unlink_or_warn(sb.buf);
|
||||
strbuf_reset(&sb);
|
||||
}
|
||||
|
||||
- connect_work_tree_and_git_dir(clone_data->path, sm_gitdir, 0);
|
||||
+ connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
|
||||
|
||||
- p = git_pathdup_submodule(clone_data->path, "config");
|
||||
+ p = git_pathdup_submodule(clone_data_path, "config");
|
||||
if (!p)
|
||||
- die(_("could not get submodule directory for '%s'"), clone_data->path);
|
||||
+ die(_("could not get submodule directory for '%s'"), clone_data_path);
|
||||
|
||||
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
|
||||
git_config_get_string("submodule.alternateLocation", &sm_alternate);
|
||||
@@ -1880,6 +1882,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
|
||||
{
|
||||
int dissociate = 0, quiet = 0, progress = 0, require_init = 0;
|
||||
struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;
|
||||
+ struct string_list reference = STRING_LIST_INIT_NODUP;
|
||||
|
||||
struct option module_clone_options[] = {
|
||||
OPT_STRING(0, "prefix", &clone_data.prefix,
|
||||
@@ -1894,7 +1897,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
|
||||
OPT_STRING(0, "url", &clone_data.url,
|
||||
N_("string"),
|
||||
N_("url where to clone the submodule from")),
|
||||
- OPT_STRING_LIST(0, "reference", &clone_data.reference,
|
||||
+ OPT_STRING_LIST(0, "reference", &reference,
|
||||
N_("repo"),
|
||||
N_("reference repository")),
|
||||
OPT_BOOL(0, "dissociate", &dissociate,
|
||||
@@ -1932,7 +1935,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
|
||||
usage_with_options(git_submodule_helper_usage,
|
||||
module_clone_options);
|
||||
|
||||
- clone_submodule(&clone_data);
|
||||
+ clone_submodule(&clone_data, &reference);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -2805,6 +2808,7 @@ static int add_submodule(const struct add_data *add_data)
|
||||
{
|
||||
char *submod_gitdir_path;
|
||||
struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;
|
||||
+ struct string_list reference = STRING_LIST_INIT_NODUP;
|
||||
|
||||
/* perhaps the path already exists and is already a git repo, else clone it */
|
||||
if (is_directory(add_data->sm_path)) {
|
||||
@@ -2821,6 +2825,7 @@ static int add_submodule(const struct add_data *add_data)
|
||||
free(submod_gitdir_path);
|
||||
} else {
|
||||
struct child_process cp = CHILD_PROCESS_INIT;
|
||||
+
|
||||
submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
|
||||
|
||||
if (is_directory(submod_gitdir_path)) {
|
||||
@@ -2852,13 +2857,13 @@ static int add_submodule(const struct add_data *add_data)
|
||||
clone_data.quiet = add_data->quiet;
|
||||
clone_data.progress = add_data->progress;
|
||||
if (add_data->reference_path)
|
||||
- string_list_append(&clone_data.reference,
|
||||
+ string_list_append(&reference,
|
||||
xstrdup(add_data->reference_path));
|
||||
clone_data.dissociate = add_data->dissociate;
|
||||
if (add_data->depth >= 0)
|
||||
clone_data.depth = xstrfmt("%d", add_data->depth);
|
||||
|
||||
- if (clone_submodule(&clone_data))
|
||||
+ if (clone_submodule(&clone_data, &reference))
|
||||
return -1;
|
||||
|
||||
prepare_submodule_repo_env(&cp.env_array);
|
||||
--
|
||||
2.20.1
|
||||
|
||||
@ -1,146 +0,0 @@
|
||||
From 41b5e59b703f94365bdd5cd6597ee150d946171b Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
|
||||
<avarab@gmail.com>
|
||||
Date: Thu, 1 Sep 2022 01:14:08 +0200
|
||||
Subject: [PATCH 2/3] submodule--helper: fix a leak in "clone_submodule"
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Fix a memory leak of the "clone_data_path" variable that we copy or
|
||||
derive from the "struct module_clone_data" in clone_submodule(). This
|
||||
code was refactored in preceding commits, but the leak has been with
|
||||
us since f8eaa0ba98b (submodule--helper, module_clone: always operate
|
||||
on absolute paths, 2016-03-31).
|
||||
|
||||
For the "else" case we don't need to xstrdup() the "clone_data->path",
|
||||
and we don't need to free our own "clone_data_path". We can therefore
|
||||
assign the "clone_data->path" to our own "clone_data_path" right away,
|
||||
and only override it (and remember to free it!) if we need to
|
||||
xstrfmt() a replacement.
|
||||
|
||||
In the case of the module_clone() caller it's from "argv", and doesn't
|
||||
need to be free'd, and in the case of the add_submodule() caller we
|
||||
get a pointer to "sm_path", which doesn't need to be directly free'd
|
||||
either.
|
||||
|
||||
Fixing this leak makes several tests pass, so let's mark them as
|
||||
passing with TEST_PASSES_SANITIZE_LEAK=true.
|
||||
|
||||
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
|
||||
Reviewed-by: Glen Choo <chooglen@google.com>
|
||||
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||||
|
||||
Reference: https://git.kernel.org/pub/scm/git/git.git/commit/?id=e77b3da6bb60e9af5963c9e42442afe53af1780b
|
||||
Conflicts:
|
||||
builtin/submodule--helper.c
|
||||
Signed-off-by: qiaojijun <qiaojijun@kylinos.cn>
|
||||
---
|
||||
builtin/submodule--helper.c | 10 +++++-----
|
||||
t/t1500-rev-parse.sh | 1 +
|
||||
t/t6008-rev-list-submodule.sh | 1 +
|
||||
t/t7414-submodule-mistakes.sh | 2 ++
|
||||
t/t7506-status-submodule.sh | 1 +
|
||||
t/t7507-commit-verbose.sh | 2 ++
|
||||
6 files changed, 12 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
|
||||
index 78ac555..bb0834a 100644
|
||||
--- a/builtin/submodule--helper.c
|
||||
+++ b/builtin/submodule--helper.c
|
||||
@@ -1784,7 +1784,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
char *sm_alternate = NULL, *error_strategy = NULL;
|
||||
struct strbuf sb = STRBUF_INIT;
|
||||
struct child_process cp = CHILD_PROCESS_INIT;
|
||||
- const char *clone_data_path;
|
||||
+ const char *clone_data_path = clone_data->path;
|
||||
+ char *to_free = NULL;
|
||||
|
||||
strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), clone_data->name);
|
||||
sm_gitdir = absolute_pathdup(sb.buf);
|
||||
@@ -1792,10 +1793,8 @@ static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
|
||||
if (!is_absolute_path(clone_data->path)) {
|
||||
strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
|
||||
- clone_data_path = xstrfmt("%s/%s", get_git_work_tree(),
|
||||
- clone_data->path);
|
||||
- } else {
|
||||
- clone_data_path = xstrdup(clone_data->path);
|
||||
+ clone_data_path = to_free = xstrfmt("%s/%s", get_git_work_tree(),
|
||||
+ clone_data->path);
|
||||
}
|
||||
|
||||
if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
|
||||
@@ -1875,6 +1874,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
strbuf_release(&sb);
|
||||
free(sm_gitdir);
|
||||
free(p);
|
||||
+ free(to_free);
|
||||
return 0;
|
||||
}
|
||||
|
||||
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
|
||||
index 1c2df08..0e13bcb 100755
|
||||
--- a/t/t1500-rev-parse.sh
|
||||
+++ b/t/t1500-rev-parse.sh
|
||||
@@ -4,6 +4,7 @@ test_description='test git rev-parse'
|
||||
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
|
||||
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
|
||||
|
||||
+TEST_PASSES_SANITIZE_LEAK=true
|
||||
. ./test-lib.sh
|
||||
|
||||
test_one () {
|
||||
diff --git a/t/t6008-rev-list-submodule.sh b/t/t6008-rev-list-submodule.sh
|
||||
index 3153a0d..12e67e1 100755
|
||||
--- a/t/t6008-rev-list-submodule.sh
|
||||
+++ b/t/t6008-rev-list-submodule.sh
|
||||
@@ -8,6 +8,7 @@ test_description='git rev-list involving submodules that this repo has'
|
||||
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
|
||||
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
|
||||
|
||||
+TEST_PASSES_SANITIZE_LEAK=true
|
||||
. ./test-lib.sh
|
||||
|
||||
test_expect_success 'setup' '
|
||||
diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh
|
||||
index f2e7df5..3269298 100755
|
||||
--- a/t/t7414-submodule-mistakes.sh
|
||||
+++ b/t/t7414-submodule-mistakes.sh
|
||||
@@ -1,6 +1,8 @@
|
||||
#!/bin/sh
|
||||
|
||||
test_description='handling of common mistakes people may make with submodules'
|
||||
+
|
||||
+TEST_PASSES_SANITIZE_LEAK=true
|
||||
. ./test-lib.sh
|
||||
|
||||
test_expect_success 'create embedded repository' '
|
||||
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
|
||||
index 3fcb447..f5426a8 100755
|
||||
--- a/t/t7506-status-submodule.sh
|
||||
+++ b/t/t7506-status-submodule.sh
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
test_description='git status for submodule'
|
||||
|
||||
+TEST_PASSES_SANITIZE_LEAK=true
|
||||
. ./test-lib.sh
|
||||
|
||||
test_create_repo_with_commit () {
|
||||
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
|
||||
index ed2653d..92462a2 100755
|
||||
--- a/t/t7507-commit-verbose.sh
|
||||
+++ b/t/t7507-commit-verbose.sh
|
||||
@@ -1,6 +1,8 @@
|
||||
#!/bin/sh
|
||||
|
||||
test_description='verbose commit template'
|
||||
+
|
||||
+TEST_PASSES_SANITIZE_LEAK=true
|
||||
. ./test-lib.sh
|
||||
|
||||
write_script "check-for-diff" <<\EOF &&
|
||||
--
|
||||
2.20.1
|
||||
|
||||
@ -1,156 +0,0 @@
|
||||
From e25bf36fdf54524bda9ec12a851ef53f43c45904 Mon Sep 17 00:00:00 2001
|
||||
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Date: Fri, 22 Mar 2024 11:19:22 +0100
|
||||
Subject: [PATCH 3/3] submodules: submodule paths must not contain symlinks
|
||||
|
||||
When creating a submodule path, we must be careful not to follow
|
||||
symbolic links. Otherwise we may follow a symbolic link pointing to
|
||||
a gitdir (which are valid symbolic links!) e.g. while cloning.
|
||||
|
||||
On case-insensitive filesystems, however, we blindly replace a directory
|
||||
that has been created as part of the `clone` operation with a symlink
|
||||
when the path to the latter differs only in case from the former's path.
|
||||
|
||||
Let's simply avoid this situation by expecting not ever having to
|
||||
overwrite any existing file/directory/symlink upon cloning. That way, we
|
||||
won't even replace a directory that we just created.
|
||||
|
||||
This addresses CVE-2024-32002.
|
||||
|
||||
Reported-by: Filip Hejsek <filip.hejsek@gmail.com>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Reference: https://git.kernel.org/pub/scm/git/git.git/commit/?id=97065761333fd62db1912d81b489db938d8c991d
|
||||
Conflicts:
|
||||
builtin/submodule--helper.c
|
||||
t/t7406-submodule-update.sh
|
||||
Signed-off-by: qiaojijun <qiaojijun@kylinos.cn>
|
||||
---
|
||||
builtin/submodule--helper.c | 35 +++++++++++++++++++++++++++
|
||||
t/t7406-submodule-update.sh | 48 +++++++++++++++++++++++++++++++++++++
|
||||
2 files changed, 83 insertions(+)
|
||||
|
||||
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
|
||||
index bb0834a..12b08ef 100644
|
||||
--- a/builtin/submodule--helper.c
|
||||
+++ b/builtin/submodule--helper.c
|
||||
@@ -1777,11 +1777,34 @@ static void prepare_possible_alternates(const char *sm_name,
|
||||
free(error_strategy);
|
||||
}
|
||||
|
||||
+static int dir_contains_only_dotgit(const char *path)
|
||||
+{
|
||||
+ DIR *dir = opendir(path);
|
||||
+ struct dirent *e;
|
||||
+ int ret = 1;
|
||||
+
|
||||
+ if (!dir)
|
||||
+ return 0;
|
||||
+
|
||||
+ e = readdir_skip_dot_and_dotdot(dir);
|
||||
+ if (!e)
|
||||
+ ret = 0;
|
||||
+ else if (strcmp(DEFAULT_GIT_DIR_ENVIRONMENT, e->d_name) ||
|
||||
+ (e = readdir_skip_dot_and_dotdot(dir))) {
|
||||
+ error("unexpected item '%s' in '%s'", e->d_name, path);
|
||||
+ ret = 0;
|
||||
+ }
|
||||
+
|
||||
+ closedir(dir);
|
||||
+ return ret;
|
||||
+}
|
||||
+
|
||||
static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
struct string_list *reference)
|
||||
{
|
||||
char *p, *sm_gitdir;
|
||||
char *sm_alternate = NULL, *error_strategy = NULL;
|
||||
+ struct stat st;
|
||||
struct strbuf sb = STRBUF_INIT;
|
||||
struct child_process cp = CHILD_PROCESS_INIT;
|
||||
const char *clone_data_path = clone_data->path;
|
||||
@@ -1802,6 +1825,10 @@ static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
"git dir"), sm_gitdir);
|
||||
|
||||
if (!file_exists(sm_gitdir)) {
|
||||
+ if (clone_data->require_init && !stat(clone_data_path, &st) &&
|
||||
+ !is_empty_dir(clone_data_path))
|
||||
+ die(_("directory not empty: '%s'"), clone_data_path);
|
||||
+
|
||||
if (safe_create_leading_directories_const(sm_gitdir) < 0)
|
||||
die(_("could not create directory '%s'"), sm_gitdir);
|
||||
|
||||
@@ -1841,6 +1868,14 @@ static int clone_submodule(const struct module_clone_data *clone_data,
|
||||
if(run_command(&cp))
|
||||
die(_("clone of '%s' into submodule path '%s' failed"),
|
||||
clone_data->url, clone_data_path);
|
||||
+
|
||||
+ if (clone_data->require_init && !stat(clone_data_path, &st) &&
|
||||
+ !dir_contains_only_dotgit(clone_data_path)) {
|
||||
+ char *dot_git = xstrfmt("%s/.git", clone_data_path);
|
||||
+ unlink(dot_git);
|
||||
+ free(dot_git);
|
||||
+ die(_("directory not empty: '%s'"), clone_data_path);
|
||||
+ }
|
||||
} else {
|
||||
if (clone_data->require_init && !access(clone_data_path, X_OK) &&
|
||||
!is_empty_dir(clone_data_path))
|
||||
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
|
||||
index 11cccbb..2d04738 100755
|
||||
--- a/t/t7406-submodule-update.sh
|
||||
+++ b/t/t7406-submodule-update.sh
|
||||
@@ -1061,4 +1061,52 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s
|
||||
)
|
||||
'
|
||||
|
||||
+test_expect_success CASE_INSENSITIVE_FS,SYMLINKS \
|
||||
+ 'submodule paths must not follow symlinks' '
|
||||
+
|
||||
+ # This is only needed because we want to run this in a self-contained
|
||||
+ # test without having to spin up an HTTP server; However, it would not
|
||||
+ # be needed in a real-world scenario where the submodule is simply
|
||||
+ # hosted on a public site.
|
||||
+ test_config_global protocol.file.allow always &&
|
||||
+
|
||||
+ # Make sure that Git tries to use symlinks on Windows
|
||||
+ test_config_global core.symlinks true &&
|
||||
+
|
||||
+ tell_tale_path="$PWD/tell.tale" &&
|
||||
+ git init hook &&
|
||||
+ (
|
||||
+ cd hook &&
|
||||
+ mkdir -p y/hooks &&
|
||||
+ write_script y/hooks/post-checkout <<-EOF &&
|
||||
+ echo HOOK-RUN >&2
|
||||
+ echo hook-run >"$tell_tale_path"
|
||||
+ EOF
|
||||
+ git add y/hooks/post-checkout &&
|
||||
+ test_tick &&
|
||||
+ git commit -m post-checkout
|
||||
+ ) &&
|
||||
+
|
||||
+ hook_repo_path="$(pwd)/hook" &&
|
||||
+ git init captain &&
|
||||
+ (
|
||||
+ cd captain &&
|
||||
+ git submodule add --name x/y "$hook_repo_path" A/modules/x &&
|
||||
+ test_tick &&
|
||||
+ git commit -m add-submodule &&
|
||||
+
|
||||
+ printf .git >dotgit.txt &&
|
||||
+ git hash-object -w --stdin <dotgit.txt >dot-git.hash &&
|
||||
+ printf "120000 %s 0\ta\n" "$(cat dot-git.hash)" >index.info &&
|
||||
+ git update-index --index-info <index.info &&
|
||||
+ test_tick &&
|
||||
+ git commit -m add-symlink
|
||||
+ ) &&
|
||||
+
|
||||
+ test_path_is_missing "$tell_tale_path" &&
|
||||
+ test_must_fail git clone --recursive captain hooked 2>err &&
|
||||
+ grep "directory not empty" err &&
|
||||
+ test_path_is_missing "$tell_tale_path"
|
||||
+'
|
||||
+
|
||||
test_done
|
||||
--
|
||||
2.20.1
|
||||
|
||||
@ -1,153 +0,0 @@
|
||||
From f4aa8c8bb11dae6e769cd930565173808cbb69c8 Mon Sep 17 00:00:00 2001
|
||||
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Date: Wed, 10 Apr 2024 14:39:37 +0200
|
||||
Subject: [PATCH] fetch/clone: detect dubious ownership of local repositories
|
||||
|
||||
When cloning from somebody else's repositories, it is possible that,
|
||||
say, the `upload-pack` command is overridden in the repository that is
|
||||
about to be cloned, which would then be run in the user's context who
|
||||
started the clone.
|
||||
|
||||
To remind the user that this is a potentially unsafe operation, let's
|
||||
extend the ownership checks we have already established for regular
|
||||
gitdir discovery to extend also to local repositories that are about to
|
||||
be cloned.
|
||||
|
||||
This protection extends also to file:// URLs.
|
||||
|
||||
The fixes in this commit address CVE-2024-32004.
|
||||
|
||||
Note: This commit does not touch the `fetch`/`clone` code directly, but
|
||||
instead the function used implicitly by both: `enter_repo()`. This
|
||||
function is also used by `git receive-pack` (i.e. pushes), by `git
|
||||
upload-archive`, by `git daemon` and by `git http-backend`. In setups
|
||||
that want to serve repositories owned by different users than the
|
||||
account running the service, this will require `safe.*` settings to be
|
||||
configured accordingly.
|
||||
|
||||
Also note: there are tiny time windows where a time-of-check-time-of-use
|
||||
("TOCTOU") race is possible. The real solution to those would be to work
|
||||
with `fstat()` and `openat()`. However, the latter function is not
|
||||
available on Windows (and would have to be emulated with rather
|
||||
expensive low-level `NtCreateFile()` calls), and the changes would be
|
||||
quite extensive, for my taste too extensive for the little gain given
|
||||
that embargoed releases need to pay extra attention to avoid introducing
|
||||
inadvertent bugs.
|
||||
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
cache.h | 12 ++++++++++++
|
||||
path.c | 2 ++
|
||||
setup.c | 21 +++++++++++++++++++++
|
||||
t/t0411-clone-from-partial.sh | 6 +++---
|
||||
4 files changed, 38 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/cache.h b/cache.h
|
||||
index fcf49706a..a46a3e4b6 100644
|
||||
--- a/cache.h
|
||||
+++ b/cache.h
|
||||
@@ -606,6 +606,18 @@ void set_git_work_tree(const char *tree);
|
||||
|
||||
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
|
||||
|
||||
+/*
|
||||
+ * Check if a repository is safe and die if it is not, by verifying the
|
||||
+ * ownership of the worktree (if any), the git directory, and the gitfile (if
|
||||
+ * any).
|
||||
+ *
|
||||
+ * Exemptions for known-safe repositories can be added via `safe.directory`
|
||||
+ * config settings; for non-bare repositories, their worktree needs to be
|
||||
+ * added, for bare ones their git directory.
|
||||
+ */
|
||||
+void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
|
||||
+ const char *gitdir);
|
||||
+
|
||||
void setup_work_tree(void);
|
||||
/*
|
||||
* Find the commondir and gitdir of the repository that contains the current
|
||||
diff --git a/path.c b/path.c
|
||||
index 492e17ad1..d61f70e87 100644
|
||||
--- a/path.c
|
||||
+++ b/path.c
|
||||
@@ -840,6 +840,7 @@ const char *enter_repo(const char *path, int strict)
|
||||
if (!suffix[i])
|
||||
return NULL;
|
||||
gitfile = read_gitfile(used_path.buf);
|
||||
+ die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
|
||||
if (gitfile) {
|
||||
strbuf_reset(&used_path);
|
||||
strbuf_addstr(&used_path, gitfile);
|
||||
@@ -850,6 +851,7 @@ const char *enter_repo(const char *path, int strict)
|
||||
}
|
||||
else {
|
||||
const char *gitfile = read_gitfile(path);
|
||||
+ die_upon_dubious_ownership(gitfile, NULL, path);
|
||||
if (gitfile)
|
||||
path = gitfile;
|
||||
if (chdir(path))
|
||||
diff --git a/setup.c b/setup.c
|
||||
index cefd5f63c..9d401ae4c 100644
|
||||
--- a/setup.c
|
||||
+++ b/setup.c
|
||||
@@ -1165,6 +1165,27 @@ static int ensure_valid_ownership(const char *gitfile,
|
||||
return data.is_safe;
|
||||
}
|
||||
|
||||
+void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
|
||||
+ const char *gitdir)
|
||||
+{
|
||||
+ struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
|
||||
+ const char *path;
|
||||
+
|
||||
+ if (ensure_valid_ownership(gitfile, worktree, gitdir, &report))
|
||||
+ return;
|
||||
+
|
||||
+ strbuf_complete(&report, '\n');
|
||||
+ path = gitfile ? gitfile : gitdir;
|
||||
+ sq_quote_buf_pretty("ed, path);
|
||||
+
|
||||
+ die(_("detected dubious ownership in repository at '%s'\n"
|
||||
+ "%s"
|
||||
+ "To add an exception for this directory, call:\n"
|
||||
+ "\n"
|
||||
+ "\tgit config --global --add safe.directory %s"),
|
||||
+ path, report.buf, quoted.buf);
|
||||
+}
|
||||
+
|
||||
enum discovery_result {
|
||||
GIT_DIR_NONE = 0,
|
||||
GIT_DIR_EXPLICIT,
|
||||
diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh
|
||||
index fb72a0a9f..eb3360dbc 100755
|
||||
--- a/t/t0411-clone-from-partial.sh
|
||||
+++ b/t/t0411-clone-from-partial.sh
|
||||
@@ -23,7 +23,7 @@ test_expect_success 'create evil repo' '
|
||||
>evil/.git/shallow
|
||||
'
|
||||
|
||||
-test_expect_failure 'local clone must not fetch from promisor remote and execute script' '
|
||||
+test_expect_success 'local clone must not fetch from promisor remote and execute script' '
|
||||
rm -f script-executed &&
|
||||
test_must_fail git clone \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
@@ -32,7 +32,7 @@ test_expect_failure 'local clone must not fetch from promisor remote and execute
|
||||
test_path_is_missing script-executed
|
||||
'
|
||||
|
||||
-test_expect_failure 'clone from file://... must not fetch from promisor remote and execute script' '
|
||||
+test_expect_success 'clone from file://... must not fetch from promisor remote and execute script' '
|
||||
rm -f script-executed &&
|
||||
test_must_fail git clone \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
@@ -41,7 +41,7 @@ test_expect_failure 'clone from file://... must not fetch from promisor remote a
|
||||
test_path_is_missing script-executed
|
||||
'
|
||||
|
||||
-test_expect_failure 'fetch from file://... must not fetch from promisor remote and execute script' '
|
||||
+test_expect_success 'fetch from file://... must not fetch from promisor remote and execute script' '
|
||||
rm -f script-executed &&
|
||||
test_must_fail git fetch \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,47 +0,0 @@
|
||||
From d51e1dff980b9fc87002436b6ab36120a39816b1 Mon Sep 17 00:00:00 2001
|
||||
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Date: Mon, 8 Aug 2022 13:27:46 +0000
|
||||
Subject: [PATCH] setup: fix some formatting
|
||||
|
||||
In preparation for touching code that was introduced in 3b0bf2704980
|
||||
(setup: tighten ownership checks post CVE-2022-24765, 2022-05-10) and
|
||||
that was formatted differently than preferred in the Git project, fix
|
||||
the indentation before actually modifying the code.
|
||||
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||||
---
|
||||
setup.c | 9 +++++----
|
||||
1 file changed, 5 insertions(+), 4 deletions(-)
|
||||
|
||||
diff --git a/setup.c b/setup.c
|
||||
index 7f64f3447..0fdecec32 100644
|
||||
--- a/setup.c
|
||||
+++ b/setup.c
|
||||
@@ -1138,7 +1138,7 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
|
||||
* added, for bare ones their git directory.
|
||||
*/
|
||||
static int ensure_valid_ownership(const char *gitfile,
|
||||
- const char *worktree, const char *gitdir)
|
||||
+ const char *worktree, const char *gitdir)
|
||||
{
|
||||
struct safe_directory_data data = {
|
||||
.path = worktree ? worktree : gitdir
|
||||
@@ -1271,10 +1271,11 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
|
||||
strbuf_setlen(dir, offset);
|
||||
if (gitdirenv) {
|
||||
enum discovery_result ret;
|
||||
+ const char *gitdir_candidate =
|
||||
+ gitdir_path ? gitdir_path : gitdirenv;
|
||||
|
||||
- if (ensure_valid_ownership(gitfile,
|
||||
- dir->buf,
|
||||
- (gitdir_path ? gitdir_path : gitdirenv))) {
|
||||
+ if (ensure_valid_ownership(gitfile, dir->buf,
|
||||
+ gitdir_candidate)) {
|
||||
strbuf_addstr(gitdir, gitdirenv);
|
||||
ret = GIT_DIR_DISCOVERED;
|
||||
} else
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,182 +0,0 @@
|
||||
From 17d3883fe9c88b823002ad9fafb42313ddc3d3d5 Mon Sep 17 00:00:00 2001
|
||||
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Date: Mon, 8 Aug 2022 13:27:47 +0000
|
||||
Subject: [PATCH] setup: prepare for more detailed "dubious ownership" messages
|
||||
|
||||
When verifying the ownership of the Git directory, we sometimes would
|
||||
like to say a bit more about it, e.g. when using a platform-dependent
|
||||
code path (think: Windows has the permission model that is so different
|
||||
from Unix'), but only when it is a appropriate to actually say
|
||||
something.
|
||||
|
||||
To allow for that, collect that information and hand it back to the
|
||||
caller (whose responsibility it is to show it or not).
|
||||
|
||||
Note: We do not actually fill in any platform-dependent information yet,
|
||||
this commit just adds the infrastructure to be able to do so.
|
||||
|
||||
Based-on-an-idea-by: Junio C Hamano <gitster@pobox.com>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||||
---
|
||||
compat/mingw.c | 2 +-
|
||||
compat/mingw.h | 2 +-
|
||||
git-compat-util.h | 5 ++++-
|
||||
setup.c | 25 +++++++++++++++----------
|
||||
4 files changed, 21 insertions(+), 13 deletions(-)
|
||||
|
||||
diff --git a/compat/mingw.c b/compat/mingw.c
|
||||
index 2607de93a..f12b7df16 100644
|
||||
--- a/compat/mingw.c
|
||||
+++ b/compat/mingw.c
|
||||
@@ -2673,7 +2673,7 @@ static PSID get_current_user_sid(void)
|
||||
return result;
|
||||
}
|
||||
|
||||
-int is_path_owned_by_current_sid(const char *path)
|
||||
+int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
|
||||
{
|
||||
WCHAR wpath[MAX_PATH];
|
||||
PSID sid = NULL;
|
||||
diff --git a/compat/mingw.h b/compat/mingw.h
|
||||
index a74da68f3..209cf7ceb 100644
|
||||
--- a/compat/mingw.h
|
||||
+++ b/compat/mingw.h
|
||||
@@ -463,7 +463,7 @@ char *mingw_query_user_email(void);
|
||||
* Verifies that the specified path is owned by the user running the
|
||||
* current process.
|
||||
*/
|
||||
-int is_path_owned_by_current_sid(const char *path);
|
||||
+int is_path_owned_by_current_sid(const char *path, struct strbuf *report);
|
||||
#define is_path_owned_by_current_user is_path_owned_by_current_sid
|
||||
|
||||
/**
|
||||
diff --git a/git-compat-util.h b/git-compat-util.h
|
||||
index 58d770829..36a25ae25 100644
|
||||
--- a/git-compat-util.h
|
||||
+++ b/git-compat-util.h
|
||||
@@ -23,6 +23,9 @@
|
||||
#include <crtdbg.h>
|
||||
#endif
|
||||
|
||||
+struct strbuf;
|
||||
+
|
||||
+
|
||||
#define _FILE_OFFSET_BITS 64
|
||||
|
||||
|
||||
@@ -487,7 +490,7 @@ static inline void extract_id_from_env(const char *env, uid_t *id)
|
||||
#endif
|
||||
|
||||
#ifndef is_path_owned_by_current_user
|
||||
-static inline int is_path_owned_by_current_uid(const char *path)
|
||||
+static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report)
|
||||
{
|
||||
struct stat st;
|
||||
if (lstat(path, &st))
|
||||
diff --git a/setup.c b/setup.c
|
||||
index 0fdecec32..ddcf6eb60 100644
|
||||
--- a/setup.c
|
||||
+++ b/setup.c
|
||||
@@ -1138,16 +1138,17 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
|
||||
* added, for bare ones their git directory.
|
||||
*/
|
||||
static int ensure_valid_ownership(const char *gitfile,
|
||||
- const char *worktree, const char *gitdir)
|
||||
+ const char *worktree, const char *gitdir,
|
||||
+ struct strbuf *report)
|
||||
{
|
||||
struct safe_directory_data data = {
|
||||
.path = worktree ? worktree : gitdir
|
||||
};
|
||||
|
||||
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
|
||||
- (!gitfile || is_path_owned_by_current_user(gitfile)) &&
|
||||
- (!worktree || is_path_owned_by_current_user(worktree)) &&
|
||||
- (!gitdir || is_path_owned_by_current_user(gitdir)))
|
||||
+ (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
|
||||
+ (!worktree || is_path_owned_by_current_user(worktree, report)) &&
|
||||
+ (!gitdir || is_path_owned_by_current_user(gitdir, report)))
|
||||
return 1;
|
||||
|
||||
/*
|
||||
@@ -1187,6 +1188,7 @@ enum discovery_result {
|
||||
*/
|
||||
static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
|
||||
struct strbuf *gitdir,
|
||||
+ struct strbuf *report,
|
||||
int die_on_error)
|
||||
{
|
||||
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
|
||||
@@ -1275,7 +1277,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
|
||||
gitdir_path ? gitdir_path : gitdirenv;
|
||||
|
||||
if (ensure_valid_ownership(gitfile, dir->buf,
|
||||
- gitdir_candidate)) {
|
||||
+ gitdir_candidate, report)) {
|
||||
strbuf_addstr(gitdir, gitdirenv);
|
||||
ret = GIT_DIR_DISCOVERED;
|
||||
} else
|
||||
@@ -1298,7 +1300,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
|
||||
}
|
||||
|
||||
if (is_git_directory(dir->buf)) {
|
||||
- if (!ensure_valid_ownership(NULL, NULL, dir->buf))
|
||||
+ if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
|
||||
return GIT_DIR_INVALID_OWNERSHIP;
|
||||
strbuf_addstr(gitdir, ".");
|
||||
return GIT_DIR_BARE;
|
||||
@@ -1331,7 +1333,7 @@ int discover_git_directory(struct strbuf *commondir,
|
||||
return -1;
|
||||
|
||||
cwd_len = dir.len;
|
||||
- if (setup_git_directory_gently_1(&dir, gitdir, 0) <= 0) {
|
||||
+ if (setup_git_directory_gently_1(&dir, gitdir, NULL, 0) <= 0) {
|
||||
strbuf_release(&dir);
|
||||
return -1;
|
||||
}
|
||||
@@ -1378,7 +1380,7 @@ int discover_git_directory(struct strbuf *commondir,
|
||||
const char *setup_git_directory_gently(int *nongit_ok)
|
||||
{
|
||||
static struct strbuf cwd = STRBUF_INIT;
|
||||
- struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;
|
||||
+ struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
|
||||
const char *prefix = NULL;
|
||||
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
|
||||
|
||||
@@ -1403,7 +1405,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
|
||||
die_errno(_("Unable to read current working directory"));
|
||||
strbuf_addbuf(&dir, &cwd);
|
||||
|
||||
- switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
|
||||
+ switch (setup_git_directory_gently_1(&dir, &gitdir, &report, 1)) {
|
||||
case GIT_DIR_EXPLICIT:
|
||||
prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);
|
||||
break;
|
||||
@@ -1435,12 +1437,14 @@ const char *setup_git_directory_gently(int *nongit_ok)
|
||||
if (!nongit_ok) {
|
||||
struct strbuf quoted = STRBUF_INIT;
|
||||
|
||||
+ strbuf_complete(&report, '\n');
|
||||
sq_quote_buf_pretty("ed, dir.buf);
|
||||
die(_("detected dubious ownership in repository at '%s'\n"
|
||||
+ "%s"
|
||||
"To add an exception for this directory, call:\n"
|
||||
"\n"
|
||||
"\tgit config --global --add safe.directory %s"),
|
||||
- dir.buf, quoted.buf);
|
||||
+ dir.buf, report.buf, quoted.buf);
|
||||
}
|
||||
*nongit_ok = 1;
|
||||
break;
|
||||
@@ -1519,6 +1523,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
|
||||
|
||||
strbuf_release(&dir);
|
||||
strbuf_release(&gitdir);
|
||||
+ strbuf_release(&report);
|
||||
clear_repository_format(&repo_fmt);
|
||||
|
||||
return prefix;
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,90 +0,0 @@
|
||||
From 5c5a4a1c05932378d259b1fdd9526cab971656a2 Mon Sep 17 00:00:00 2001
|
||||
From: Filip Hejsek <filip.hejsek@gmail.com>
|
||||
Date: Sun, 28 Jan 2024 04:29:33 +0100
|
||||
Subject: [PATCH] t0411: add tests for cloning from partial repo
|
||||
|
||||
Cloning from a partial repository must not fetch missing objects into
|
||||
the partial repository, because that can lead to arbitrary code
|
||||
execution.
|
||||
|
||||
Add a couple of test cases, pretending to the `upload-pack` command (and
|
||||
to that command only) that it is working on a repository owned by
|
||||
someone else.
|
||||
|
||||
Helped-by: Jeff King <peff@peff.net>
|
||||
Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
t/t0411-clone-from-partial.sh | 60 +++++++++++++++++++++++++++++++++++
|
||||
1 file changed, 60 insertions(+)
|
||||
create mode 100755 t/t0411-clone-from-partial.sh
|
||||
|
||||
diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh
|
||||
new file mode 100755
|
||||
index 000000000..fb72a0a9f
|
||||
--- /dev/null
|
||||
+++ b/t/t0411-clone-from-partial.sh
|
||||
@@ -0,0 +1,60 @@
|
||||
+#!/bin/sh
|
||||
+
|
||||
+test_description='check that local clone does not fetch from promisor remotes'
|
||||
+
|
||||
+. ./test-lib.sh
|
||||
+
|
||||
+test_expect_success 'create evil repo' '
|
||||
+ git init tmp &&
|
||||
+ test_commit -C tmp a &&
|
||||
+ git -C tmp config uploadpack.allowfilter 1 &&
|
||||
+ git clone --filter=blob:none --no-local --no-checkout tmp evil &&
|
||||
+ rm -rf tmp &&
|
||||
+
|
||||
+ git -C evil config remote.origin.uploadpack \"\$TRASH_DIRECTORY/fake-upload-pack\" &&
|
||||
+ write_script fake-upload-pack <<-\EOF &&
|
||||
+ echo >&2 "fake-upload-pack running"
|
||||
+ >"$TRASH_DIRECTORY/script-executed"
|
||||
+ exit 1
|
||||
+ EOF
|
||||
+ export TRASH_DIRECTORY &&
|
||||
+
|
||||
+ # empty shallow file disables local clone optimization
|
||||
+ >evil/.git/shallow
|
||||
+'
|
||||
+
|
||||
+test_expect_failure 'local clone must not fetch from promisor remote and execute script' '
|
||||
+ rm -f script-executed &&
|
||||
+ test_must_fail git clone \
|
||||
+ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
+ evil clone1 2>err &&
|
||||
+ ! grep "fake-upload-pack running" err &&
|
||||
+ test_path_is_missing script-executed
|
||||
+'
|
||||
+
|
||||
+test_expect_failure 'clone from file://... must not fetch from promisor remote and execute script' '
|
||||
+ rm -f script-executed &&
|
||||
+ test_must_fail git clone \
|
||||
+ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
+ "file://$(pwd)/evil" clone2 2>err &&
|
||||
+ ! grep "fake-upload-pack running" err &&
|
||||
+ test_path_is_missing script-executed
|
||||
+'
|
||||
+
|
||||
+test_expect_failure 'fetch from file://... must not fetch from promisor remote and execute script' '
|
||||
+ rm -f script-executed &&
|
||||
+ test_must_fail git fetch \
|
||||
+ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
+ "file://$(pwd)/evil" 2>err &&
|
||||
+ ! grep "fake-upload-pack running" err &&
|
||||
+ test_path_is_missing script-executed
|
||||
+'
|
||||
+
|
||||
+test_expect_success 'pack-objects should fetch from promisor remote and execute script' '
|
||||
+ rm -f script-executed &&
|
||||
+ echo "HEAD" | test_must_fail git -C evil pack-objects --revs --stdout >/dev/null 2>err &&
|
||||
+ grep "fake-upload-pack running" err &&
|
||||
+ test_path_is_file script-executed
|
||||
+'
|
||||
+
|
||||
+test_done
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,109 +0,0 @@
|
||||
From 1204e1a824c34071019fe106348eaa6d88f9528d Mon Sep 17 00:00:00 2001
|
||||
From: Patrick Steinhardt <ps@pks.im>
|
||||
Date: Mon, 15 Apr 2024 13:30:41 +0200
|
||||
Subject: [PATCH] builtin/clone: refuse local clones of unsafe repositories
|
||||
|
||||
When performing a local clone of a repository we end up either copying
|
||||
or hardlinking the source repository into the target repository. This is
|
||||
significantly more performant than if we were to use git-upload-pack(1)
|
||||
and git-fetch-pack(1) to create the new repository and preserves both
|
||||
disk space and compute time.
|
||||
|
||||
Unfortunately though, performing such a local clone of a repository that
|
||||
is not owned by the current user is inherently unsafe:
|
||||
|
||||
- It is possible that source files get swapped out underneath us while
|
||||
we are copying or hardlinking them. While we do perform some checks
|
||||
here to assert that we hardlinked the expected file, they cannot
|
||||
reliably thwart time-of-check-time-of-use (TOCTOU) style races. It
|
||||
is thus possible for an adversary to make us copy or hardlink
|
||||
unexpected files into the target directory.
|
||||
|
||||
Ideally, we would address this by starting to use openat(3P),
|
||||
fstatat(3P) and friends. Due to platform compatibility with Windows
|
||||
we cannot easily do that though. Furthermore, the scope of these
|
||||
fixes would likely be quite broad and thus not fit for an embargoed
|
||||
security release.
|
||||
|
||||
- Even if we handled TOCTOU-style races perfectly, hardlinking files
|
||||
owned by a different user into the target repository is not a good
|
||||
idea in general. It is possible for an adversary to rewrite those
|
||||
files to contain whatever data they want even after the clone has
|
||||
completed.
|
||||
|
||||
Address these issues by completely refusing local clones of a repository
|
||||
that is not owned by the current user. This reuses our existing infra we
|
||||
have in place via `ensure_valid_ownership()` and thus allows a user to
|
||||
override the safety guard by adding the source repository path to the
|
||||
"safe.directory" configuration.
|
||||
|
||||
This addresses CVE-2024-32020.
|
||||
|
||||
Signed-off-by: Patrick Steinhardt <ps@pks.im>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
builtin/clone.c | 14 ++++++++++++++
|
||||
t/t0033-safe-directory.sh | 24 ++++++++++++++++++++++++
|
||||
2 files changed, 38 insertions(+)
|
||||
|
||||
diff --git a/builtin/clone.c b/builtin/clone.c
|
||||
index 4b80fa087..9ec500d42 100644
|
||||
--- a/builtin/clone.c
|
||||
+++ b/builtin/clone.c
|
||||
@@ -321,6 +321,20 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
||||
struct dir_iterator *iter;
|
||||
int iter_status;
|
||||
|
||||
+ /*
|
||||
+ * Refuse copying directories by default which aren't owned by us. The
|
||||
+ * code that performs either the copying or hardlinking is not prepared
|
||||
+ * to handle various edge cases where an adversary may for example
|
||||
+ * racily swap out files for symlinks. This can cause us to
|
||||
+ * inadvertently use the wrong source file.
|
||||
+ *
|
||||
+ * Furthermore, even if we were prepared to handle such races safely,
|
||||
+ * creating hardlinks across user boundaries is an inherently unsafe
|
||||
+ * operation as the hardlinked files can be rewritten at will by the
|
||||
+ * potentially-untrusted user. We thus refuse to do so by default.
|
||||
+ */
|
||||
+ die_upon_dubious_ownership(NULL, NULL, src_repo);
|
||||
+
|
||||
mkdir_if_missing(dest->buf, 0777);
|
||||
|
||||
iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);
|
||||
diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
|
||||
index dc3496897..11c3e8f28 100755
|
||||
--- a/t/t0033-safe-directory.sh
|
||||
+++ b/t/t0033-safe-directory.sh
|
||||
@@ -80,4 +80,28 @@ test_expect_success 'safe.directory=*, but is reset' '
|
||||
expect_rejected_dir
|
||||
'
|
||||
|
||||
+test_expect_success 'local clone of unowned repo refused in unsafe directory' '
|
||||
+ test_when_finished "rm -rf source" &&
|
||||
+ git init source &&
|
||||
+ (
|
||||
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
|
||||
+ test_commit -C source initial
|
||||
+ ) &&
|
||||
+ test_must_fail git clone --local source target &&
|
||||
+ test_path_is_missing target
|
||||
+'
|
||||
+
|
||||
+test_expect_success 'local clone of unowned repo accepted in safe directory' '
|
||||
+ test_when_finished "rm -rf source" &&
|
||||
+ git init source &&
|
||||
+ (
|
||||
+ sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
|
||||
+ test_commit -C source initial
|
||||
+ ) &&
|
||||
+ test_must_fail git clone --local source target &&
|
||||
+ git config --global --add safe.directory "$(pwd)/source/.git" &&
|
||||
+ git clone --local source target &&
|
||||
+ test_path_is_dir target
|
||||
+'
|
||||
+
|
||||
test_done
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,60 +0,0 @@
|
||||
From d1bb66a546b4bb46005d17ba711caaad26f26c1e Mon Sep 17 00:00:00 2001
|
||||
From: Patrick Steinhardt <ps@pks.im>
|
||||
Date: Mon, 15 Apr 2024 13:30:31 +0200
|
||||
Subject: [PATCH] builtin/clone: abort when hardlinked source and target file
|
||||
differ
|
||||
|
||||
When performing local clones with hardlinks we refuse to copy source
|
||||
files which are symlinks as a mitigation for CVE-2022-39253. This check
|
||||
can be raced by an adversary though by changing the file to a symlink
|
||||
after we have checked it.
|
||||
|
||||
Fix the issue by checking whether the hardlinked destination file
|
||||
matches the source file and abort in case it doesn't.
|
||||
|
||||
This addresses CVE-2024-32021.
|
||||
|
||||
Reported-by: Apple Product Security <product-security@apple.com>
|
||||
Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org>
|
||||
Signed-off-by: Patrick Steinhardt <ps@pks.im>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
builtin/clone.c | 21 ++++++++++++++++++++-
|
||||
1 file changed, 20 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/builtin/clone.c b/builtin/clone.c
|
||||
index 073e6323d..4b80fa087 100644
|
||||
--- a/builtin/clone.c
|
||||
+++ b/builtin/clone.c
|
||||
@@ -357,8 +357,27 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
||||
if (unlink(dest->buf) && errno != ENOENT)
|
||||
die_errno(_("failed to unlink '%s'"), dest->buf);
|
||||
if (!option_no_hardlinks) {
|
||||
- if (!link(src->buf, dest->buf))
|
||||
+ if (!link(src->buf, dest->buf)) {
|
||||
+ struct stat st;
|
||||
+
|
||||
+ /*
|
||||
+ * Sanity-check whether the created hardlink
|
||||
+ * actually links to the expected file now. This
|
||||
+ * catches time-of-check-time-of-use bugs in
|
||||
+ * case the source file was meanwhile swapped.
|
||||
+ */
|
||||
+ if (lstat(dest->buf, &st))
|
||||
+ die(_("hardlink cannot be checked at '%s'"), dest->buf);
|
||||
+ if (st.st_mode != iter->st.st_mode ||
|
||||
+ st.st_ino != iter->st.st_ino ||
|
||||
+ st.st_dev != iter->st.st_dev ||
|
||||
+ st.st_size != iter->st.st_size ||
|
||||
+ st.st_uid != iter->st.st_uid ||
|
||||
+ st.st_gid != iter->st.st_gid)
|
||||
+ die(_("hardlink different from source at '%s'"), dest->buf);
|
||||
+
|
||||
continue;
|
||||
+ }
|
||||
if (option_local > 0)
|
||||
die_errno(_("failed to create link '%s'"), dest->buf);
|
||||
option_no_hardlinks = 1;
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,84 +0,0 @@
|
||||
From 150e6b0aedf57d224c3c49038c306477fa159886 Mon Sep 17 00:00:00 2001
|
||||
From: Patrick Steinhardt <ps@pks.im>
|
||||
Date: Mon, 15 Apr 2024 13:30:26 +0200
|
||||
Subject: [PATCH] builtin/clone: stop resolving symlinks when copying files
|
||||
|
||||
When a user performs a local clone without `--no-local`, then we end up
|
||||
copying the source repository into the target repository directly. To
|
||||
optimize this even further, we try to hardlink files into place instead
|
||||
of copying data over, which helps both disk usage and speed.
|
||||
|
||||
There is an important edge case in this context though, namely when we
|
||||
try to hardlink symlinks from the source repository into the target
|
||||
repository. Depending on both platform and filesystem the resulting
|
||||
behaviour here can be different:
|
||||
|
||||
- On macOS and NetBSD, calling link(3P) with a symlink target creates
|
||||
a hardlink to the file pointed to by the symlink.
|
||||
|
||||
- On Linux, calling link(3P) instead creates a hardlink to the symlink
|
||||
itself.
|
||||
|
||||
To unify this behaviour, 36596fd2df (clone: better handle symlinked
|
||||
files at .git/objects/, 2019-07-10) introduced logic to resolve symlinks
|
||||
before we try to link(3P) files. Consequently, the new behaviour was to
|
||||
always create a hard link to the target of the symlink on all platforms.
|
||||
|
||||
Eventually though, we figured out that following symlinks like this can
|
||||
cause havoc when performing a local clone of a malicious repository,
|
||||
which resulted in CVE-2022-39253. This issue was fixed via 6f054f9fb3
|
||||
(builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28),
|
||||
by refusing symlinks in the source repository.
|
||||
|
||||
But even though we now shouldn't ever link symlinks anymore, the code
|
||||
that resolves symlinks still exists. In the best case the code does not
|
||||
end up doing anything because there are no symlinks anymore. In the
|
||||
worst case though this can be abused by an adversary that rewrites the
|
||||
source file after it has been checked not to be a symlink such that it
|
||||
actually is a symlink when we call link(3P). Thus, it is still possible
|
||||
to recreate CVE-2022-39253 due to this time-of-check-time-of-use bug.
|
||||
|
||||
Remove the call to `realpath()`. This doesn't yet address the actual
|
||||
vulnerability, which will be handled in a subsequent commit.
|
||||
|
||||
Reported-by: Apple Product Security <product-security@apple.com>
|
||||
Signed-off-by: Patrick Steinhardt <ps@pks.im>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
builtin/clone.c | 6 +-----
|
||||
1 file changed, 1 insertion(+), 5 deletions(-)
|
||||
|
||||
diff --git a/builtin/clone.c b/builtin/clone.c
|
||||
index 3c2ae31a5..073e6323d 100644
|
||||
--- a/builtin/clone.c
|
||||
+++ b/builtin/clone.c
|
||||
@@ -320,7 +320,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
||||
int src_len, dest_len;
|
||||
struct dir_iterator *iter;
|
||||
int iter_status;
|
||||
- struct strbuf realpath = STRBUF_INIT;
|
||||
|
||||
mkdir_if_missing(dest->buf, 0777);
|
||||
|
||||
@@ -358,8 +357,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
||||
if (unlink(dest->buf) && errno != ENOENT)
|
||||
die_errno(_("failed to unlink '%s'"), dest->buf);
|
||||
if (!option_no_hardlinks) {
|
||||
- strbuf_realpath(&realpath, src->buf, 1);
|
||||
- if (!link(realpath.buf, dest->buf))
|
||||
+ if (!link(src->buf, dest->buf))
|
||||
continue;
|
||||
if (option_local > 0)
|
||||
die_errno(_("failed to create link '%s'"), dest->buf);
|
||||
@@ -373,8 +371,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
||||
strbuf_setlen(src, src_len);
|
||||
die(_("failed to iterate over '%s'"), src->buf);
|
||||
}
|
||||
-
|
||||
- strbuf_release(&realpath);
|
||||
}
|
||||
|
||||
static void clone_local(const char *src_repo, const char *dest_repo)
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,201 +0,0 @@
|
||||
From 7b70e9efb18c2cc3f219af399bd384c5801ba1d7 Mon Sep 17 00:00:00 2001
|
||||
From: Jeff King <peff@peff.net>
|
||||
Date: Tue, 16 Apr 2024 04:35:33 -0400
|
||||
Subject: [PATCH] upload-pack: disable lazy-fetching by default
|
||||
|
||||
The upload-pack command tries to avoid trusting the repository in which
|
||||
it's run (e.g., by not running any hooks and not using any config that
|
||||
contains arbitrary commands). But if the server side of a fetch or a
|
||||
clone is a partial clone, then either upload-pack or its child
|
||||
pack-objects may run a lazy "git fetch" under the hood. And it is very
|
||||
easy to convince fetch to run arbitrary commands.
|
||||
|
||||
The "server" side can be a local repository owned by someone else, who
|
||||
would be able to configure commands that are run during a clone with the
|
||||
current user's permissions. This issue has been designated
|
||||
CVE-2024-32004.
|
||||
|
||||
The fix in this commit's parent helps in this scenario, as well as in
|
||||
related scenarios using SSH to clone, where the untrusted .git directory
|
||||
is owned by a different user id. But if you received one as a zip file,
|
||||
on a USB stick, etc, it may be owned by your user but still untrusted.
|
||||
|
||||
This has been designated CVE-2024-32465.
|
||||
|
||||
To mitigate the issue more completely, let's disable lazy fetching
|
||||
entirely during `upload-pack`. While fetching from a partial repository
|
||||
should be relatively rare, it is certainly not an unreasonable workflow.
|
||||
And thus we need to provide an escape hatch.
|
||||
|
||||
This commit works by respecting a GIT_NO_LAZY_FETCH environment variable
|
||||
(to skip the lazy-fetch), and setting it in upload-pack, but only when
|
||||
the user has not already done so (which gives us the escape hatch).
|
||||
|
||||
The name of the variable is specifically chosen to match what has
|
||||
already been added in 'master' via e6d5479e7a (git: extend
|
||||
--no-lazy-fetch to work across subprocesses, 2024-02-27). Since we're
|
||||
building this fix as a backport for older versions, we could cherry-pick
|
||||
that patch and its earlier steps. However, we don't really need the
|
||||
niceties (like a "--no-lazy-fetch" option) that it offers. By using the
|
||||
same name, everything should just work when the two are eventually
|
||||
merged, but here are a few notes:
|
||||
|
||||
- the blocking of the fetch in e6d5479e7a is incomplete! It sets
|
||||
fetch_if_missing to 0 when we setup the repository variable, but
|
||||
that isn't enough. pack-objects in particular will call
|
||||
prefetch_to_pack() even if that variable is 0. This patch by
|
||||
contrast checks the environment variable at the lowest level before
|
||||
we call the lazy fetch, where we can be sure to catch all code
|
||||
paths.
|
||||
|
||||
Possibly the setting of fetch_if_missing from e6d5479e7a can be
|
||||
reverted, but it may be useful to have. For example, some code may
|
||||
want to use that flag to change behavior before it gets to the point
|
||||
of trying to start the fetch. At any rate, that's all outside the
|
||||
scope of this patch.
|
||||
|
||||
- there's documentation for GIT_NO_LAZY_FETCH in e6d5479e7a. We can
|
||||
live without that here, because for the most part the user shouldn't
|
||||
need to set it themselves. The exception is if they do want to
|
||||
override upload-pack's default, and that requires a separate
|
||||
documentation section (which is added here)
|
||||
|
||||
- it would be nice to use the NO_LAZY_FETCH_ENVIRONMENT macro added by
|
||||
e6d5479e7a, but those definitions have moved from cache.h to
|
||||
environment.h between 2.39.3 and master. I just used the raw string
|
||||
literals, and we can replace them with the macro once this topic is
|
||||
merged to master.
|
||||
|
||||
At least with respect to CVE-2024-32004, this does render this commit's
|
||||
parent commit somewhat redundant. However, it is worth retaining that
|
||||
commit as defense in depth, and because it may help other issues (e.g.,
|
||||
symlink/hardlink TOCTOU races, where zip files are not really an
|
||||
interesting attack vector).
|
||||
|
||||
The tests in t0411 still pass, but now we have _two_ mechanisms ensuring
|
||||
that the evil command is not run. Let's beef up the existing ones to
|
||||
check that they failed for the expected reason, that we refused to run
|
||||
upload-pack at all with an alternate user id. And add two new ones for
|
||||
the same-user case that both the restriction and its escape hatch.
|
||||
|
||||
Signed-off-by: Jeff King <peff@peff.net>
|
||||
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
||||
---
|
||||
Documentation/git-upload-pack.txt | 16 ++++++++++++++++
|
||||
builtin/upload-pack.c | 2 ++
|
||||
promisor-remote.c | 10 ++++++++++
|
||||
t/t0411-clone-from-partial.sh | 18 ++++++++++++++++++
|
||||
4 files changed, 46 insertions(+)
|
||||
|
||||
diff --git a/Documentation/git-upload-pack.txt b/Documentation/git-upload-pack.txt
|
||||
index b656b4756..fc4c62d7b 100644
|
||||
--- a/Documentation/git-upload-pack.txt
|
||||
+++ b/Documentation/git-upload-pack.txt
|
||||
@@ -55,6 +55,22 @@ --advertise-refs::
|
||||
<directory>::
|
||||
The repository to sync from.
|
||||
|
||||
+`GIT_NO_LAZY_FETCH`::
|
||||
+ When cloning or fetching from a partial repository (i.e., one
|
||||
+ itself cloned with `--filter`), the server-side `upload-pack`
|
||||
+ may need to fetch extra objects from its upstream in order to
|
||||
+ complete the request. By default, `upload-pack` will refuse to
|
||||
+ perform such a lazy fetch, because `git fetch` may run arbitrary
|
||||
+ commands specified in configuration and hooks of the source
|
||||
+ repository (and `upload-pack` tries to be safe to run even in
|
||||
+ untrusted `.git` directories).
|
||||
++
|
||||
+This is implemented by having `upload-pack` internally set the
|
||||
+`GIT_NO_LAZY_FETCH` variable to `1`. If you want to override it
|
||||
+(because you are fetching from a partial clone, and you are sure
|
||||
+you trust it), you can explicitly set `GIT_NO_LAZY_FETCH` to
|
||||
+`0`.
|
||||
+
|
||||
SEE ALSO
|
||||
--------
|
||||
linkgit:gitnamespaces[7]
|
||||
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
|
||||
index 25b69da2b..f446ff04f 100644
|
||||
--- a/builtin/upload-pack.c
|
||||
+++ b/builtin/upload-pack.c
|
||||
@@ -35,6 +35,8 @@ int cmd_upload_pack(int argc, const char **argv, const char *prefix)
|
||||
|
||||
packet_trace_identity("upload-pack");
|
||||
read_replace_refs = 0;
|
||||
+ /* TODO: This should use NO_LAZY_FETCH_ENVIRONMENT */
|
||||
+ xsetenv("GIT_NO_LAZY_FETCH", "1", 0);
|
||||
|
||||
argc = parse_options(argc, argv, prefix, options, upload_pack_usage, 0);
|
||||
|
||||
diff --git a/promisor-remote.c b/promisor-remote.c
|
||||
index faa761294..550a38f75 100644
|
||||
--- a/promisor-remote.c
|
||||
+++ b/promisor-remote.c
|
||||
@@ -20,6 +20,16 @@ static int fetch_objects(struct repository *repo,
|
||||
int i;
|
||||
FILE *child_in;
|
||||
|
||||
+ /* TODO: This should use NO_LAZY_FETCH_ENVIRONMENT */
|
||||
+ if (git_env_bool("GIT_NO_LAZY_FETCH", 0)) {
|
||||
+ static int warning_shown;
|
||||
+ if (!warning_shown) {
|
||||
+ warning_shown = 1;
|
||||
+ warning(_("lazy fetching disabled; some objects may not be available"));
|
||||
+ }
|
||||
+ return -1;
|
||||
+ }
|
||||
+
|
||||
child.git_cmd = 1;
|
||||
child.in = -1;
|
||||
if (repo != the_repository)
|
||||
diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh
|
||||
index eb3360dbc..b3d6ddc4b 100755
|
||||
--- a/t/t0411-clone-from-partial.sh
|
||||
+++ b/t/t0411-clone-from-partial.sh
|
||||
@@ -28,6 +28,7 @@ test_expect_success 'local clone must not fetch from promisor remote and execute
|
||||
test_must_fail git clone \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
evil clone1 2>err &&
|
||||
+ grep "detected dubious ownership" err &&
|
||||
! grep "fake-upload-pack running" err &&
|
||||
test_path_is_missing script-executed
|
||||
'
|
||||
@@ -37,6 +38,7 @@ test_expect_success 'clone from file://... must not fetch from promisor remote a
|
||||
test_must_fail git clone \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
"file://$(pwd)/evil" clone2 2>err &&
|
||||
+ grep "detected dubious ownership" err &&
|
||||
! grep "fake-upload-pack running" err &&
|
||||
test_path_is_missing script-executed
|
||||
'
|
||||
@@ -46,6 +48,7 @@ test_expect_success 'fetch from file://... must not fetch from promisor remote a
|
||||
test_must_fail git fetch \
|
||||
--upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
|
||||
"file://$(pwd)/evil" 2>err &&
|
||||
+ grep "detected dubious ownership" err &&
|
||||
! grep "fake-upload-pack running" err &&
|
||||
test_path_is_missing script-executed
|
||||
'
|
||||
@@ -57,4 +60,19 @@ test_expect_success 'pack-objects should fetch from promisor remote and execute
|
||||
test_path_is_file script-executed
|
||||
'
|
||||
|
||||
+test_expect_success 'clone from promisor remote does not lazy-fetch by default' '
|
||||
+ rm -f script-executed &&
|
||||
+ test_must_fail git clone evil no-lazy 2>err &&
|
||||
+ grep "lazy fetching disabled" err &&
|
||||
+ test_path_is_missing script-executed
|
||||
+'
|
||||
+
|
||||
+test_expect_success 'promisor lazy-fetching can be re-enabled' '
|
||||
+ rm -f script-executed &&
|
||||
+ test_must_fail env GIT_NO_LAZY_FETCH=0 \
|
||||
+ git clone evil lazy-ok 2>err &&
|
||||
+ grep "fake-upload-pack running" err &&
|
||||
+ test_path_is_file script-executed
|
||||
+'
|
||||
+
|
||||
test_done
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,108 +0,0 @@
|
||||
From 3540c71ea5ddffff6e473249866cbc7abb8ce509 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
|
||||
<avarab@gmail.com>
|
||||
Date: Tue, 21 Sep 2021 15:12:59 +0200
|
||||
Subject: [PATCH] wrapper.c: add x{un,}setenv(), and use xsetenv() in
|
||||
environment.c
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Add fatal wrappers for setenv() and unsetenv(). In d7ac12b25d3 (Add
|
||||
set_git_dir() function, 2007-08-01) we started checking its return
|
||||
value, and since 48988c4d0c3 (set_git_dir: die when setenv() fails,
|
||||
2018-03-30) we've had set_git_dir_1() die if we couldn't set it.
|
||||
|
||||
Let's provide a wrapper for both, this will be useful in many other
|
||||
places, a subsequent patch will make another use of xsetenv().
|
||||
|
||||
The checking of the return value here is over-eager according to
|
||||
setenv(3) and POSIX. It's documented as returning just -1 or 0, so
|
||||
perhaps we should be checking -1 explicitly.
|
||||
|
||||
Let's just instead die on any non-zero, if our C library is so broken
|
||||
as to return something else than -1 on error (and perhaps not set
|
||||
errno?) the worst we'll do is die with a nonsensical errno value, but
|
||||
we'll want to die in either case.
|
||||
|
||||
Let's make these return "void" instead of "int". As far as I can tell
|
||||
there's no other x*() wrappers that needed to make the decision of
|
||||
deviating from the signature in the C library, but since their return
|
||||
value is only used to indicate errors (so we'd die here), we can catch
|
||||
unreachable code such as
|
||||
|
||||
if (xsetenv(...) < 0)
|
||||
[...];
|
||||
|
||||
I think it would be OK skip the NULL check of the "name" here for the
|
||||
calls to die_errno(). Almost all of our setenv() callers are taking a
|
||||
constant string hardcoded in the source as the first argument, and for
|
||||
the rest we can probably assume they've done the NULL check
|
||||
themselves. Even if they didn't, modern C libraries are forgiving
|
||||
about it (e.g. glibc formatting it as "(null)"), on those that aren't,
|
||||
well, we were about to die anyway. But let's include the check anyway
|
||||
for good measure.
|
||||
|
||||
1. https://pubs.opengroup.org/onlinepubs/009604499/functions/setenv.html
|
||||
|
||||
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
|
||||
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
||||
---
|
||||
environment.c | 3 +--
|
||||
git-compat-util.h | 2 ++
|
||||
wrapper.c | 12 ++++++++++++
|
||||
3 files changed, 15 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/environment.c b/environment.c
|
||||
index d6b22ede7ea288..7d8a949285c101 100644
|
||||
--- a/environment.c
|
||||
+++ b/environment.c
|
||||
@@ -330,8 +330,7 @@ char *get_graft_file(struct repository *r)
|
||||
|
||||
static void set_git_dir_1(const char *path)
|
||||
{
|
||||
- if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
|
||||
- die(_("could not set GIT_DIR to '%s'"), path);
|
||||
+ xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
|
||||
setup_git_env(path);
|
||||
}
|
||||
|
||||
diff --git a/git-compat-util.h b/git-compat-util.h
|
||||
index b46605300abf81..b3ee81602c28e0 100644
|
||||
--- a/git-compat-util.h
|
||||
+++ b/git-compat-util.h
|
||||
@@ -875,6 +875,8 @@ void *xmemdupz(const void *data, size_t len);
|
||||
char *xstrndup(const char *str, size_t len);
|
||||
void *xrealloc(void *ptr, size_t size);
|
||||
void *xcalloc(size_t nmemb, size_t size);
|
||||
+void xsetenv(const char *name, const char *value, int overwrite);
|
||||
+void xunsetenv(const char *name);
|
||||
void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
|
||||
const char *mmap_os_err(void);
|
||||
void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
|
||||
diff --git a/wrapper.c b/wrapper.c
|
||||
index 7c6586af321000..1460d4e27b03cc 100644
|
||||
--- a/wrapper.c
|
||||
+++ b/wrapper.c
|
||||
@@ -145,6 +145,18 @@ void *xcalloc(size_t nmemb, size_t size)
|
||||
return ret;
|
||||
}
|
||||
|
||||
+void xsetenv(const char *name, const char *value, int overwrite)
|
||||
+{
|
||||
+ if (setenv(name, value, overwrite))
|
||||
+ die_errno(_("could not setenv '%s'"), name ? name : "(null)");
|
||||
+}
|
||||
+
|
||||
+void xunsetenv(const char *name)
|
||||
+{
|
||||
+ if (!unsetenv(name))
|
||||
+ die_errno(_("could not unsetenv '%s'"), name ? name : "(null)");
|
||||
+}
|
||||
+
|
||||
/*
|
||||
* Limit size of IO chunks, because huge chunks only cause pain. OS X
|
||||
* 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
|
||||
--
|
||||
2.33.0
|
||||
|
||||
88
git.spec
88
git.spec
@ -1,7 +1,7 @@
|
||||
%global gitexecdir %{_libexecdir}/git-core
|
||||
Name: git
|
||||
Version: 2.33.0
|
||||
Release: 15
|
||||
Release: 10
|
||||
Summary: A popular and widely used Version Control System
|
||||
License: GPLv2+ or LGPLv2.1
|
||||
URL: https://git-scm.com/
|
||||
@ -59,25 +59,13 @@ Patch43: backport-CVE-2023-23946-apply-fix-writing-behind-newly-created-symbo
|
||||
Patch44: backport-CVE-2023-25652-apply-reject-overwrite-existing-.rej-symlink-if-it-e.patch
|
||||
Patch45: backport-CVE-2023-29007.patch
|
||||
Patch46: backport-CVE-2023-25815-gettext-avoid-using-gettext-if-the-locale-dir-is-not.patch
|
||||
Patch47: backport-CVE-2024-32021-builtin-clone-stop-resolving-symlinks-when-copying-f.patch
|
||||
Patch48: backport-CVE-2024-32021-builtin-clone-abort-when-hardlinked-source-and-targe.patch
|
||||
Patch49: backport-CVE-2024-32004-t0411-add-tests-for-cloning-from-partial-repo.patch
|
||||
Patch50: backport-CVE-2024-32004-setup-fix-some-formatting.patch
|
||||
Patch51: backport-CVE-2024-32004-setup-prepare-for-more-detailed-dubious-ownership-me.patch
|
||||
Patch52: backport-CVE-2024-32004-fetch-clone-detect-dubious-ownership-of-local-reposi.patch
|
||||
Patch53: backport-CVE-2024-32020-builtin-clone-refuse-local-clones-of-unsafe-reposito.patch
|
||||
Patch54: backport-CVE-2024-32465-wrapper.c-add-x-un-setenv-and-use-xsetenv-in.patch
|
||||
Patch55: backport-CVE-2024-32465-upload-pack-disable-lazy-fetching-by-default.patch
|
||||
Patch56: backport-CVE-2024-32002-submodule-helper-add-const-to-passed-module_clone_da.patch
|
||||
Patch57: backport-CVE-2024-32002-submodule-helper-fix-a-leak-in-clone_submodule.patch
|
||||
Patch58: backport-CVE-2024-32002-submodules-submodule-paths-must-not-contain-symlinks.patch
|
||||
|
||||
BuildRequires: gcc gettext
|
||||
BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre2-devel desktop-file-utils
|
||||
BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils
|
||||
BuildRequires: python3-devel perl-generators perl-interpreter perl-Error perl(Test::More) perl-MailTools perl(Test)
|
||||
Requires: perl(Term::ReadKey) perl-Git git-core = %{version}-%{release}
|
||||
Obsoletes: %{name}-subtree < %{version}-%{release} %{name}-p4 < %{version}-%{release} git-cvs < %{version}-%{release}
|
||||
Provides: %{name} = %{version}-%{release} %{name}-subtree = %{version}-%{release} %{name}-p4 = %{version}-%{release}
|
||||
Requires: less zlib openssh-clients perl(Term::ReadKey) perl-Git git-core = %{version}-%{release}
|
||||
Obsoletes: %{name}-core-doc %{name}-subtree %{name}-p4 git-cvs < %{version}-%{release}
|
||||
Provides: %{name} <= %{version}-%{release} %{name}-subtree %{name}-p4
|
||||
|
||||
%description
|
||||
Git is a free and open source distributed version control system
|
||||
@ -133,8 +121,8 @@ Requires: %{name} = %{version}-%{release} tk
|
||||
Summary: Git web interfaces
|
||||
BuildArch: noarch
|
||||
Requires: %{name} = %{version}-%{release}
|
||||
Obsoletes: gitweb < %{version}-%{release} %{name}-instaweb < %{version}-%{release}
|
||||
Provides: gitweb = %{version}-%{release} %{name}-instaweb = %{version}-%{release}
|
||||
Obsoletes: gitweb %{name}-instaweb
|
||||
Provides: gitweb %{name}-instaweb
|
||||
|
||||
%description web
|
||||
Git web interface allows user browsing git repositories via web service.
|
||||
@ -175,8 +163,8 @@ Requires: git = %{version}-%{release} perl(:MODULE_COMPAT_%(perl -V:versi
|
||||
%package help
|
||||
Summary: Man pages and documents for Git system
|
||||
BuildArch: noarch
|
||||
Obsoletes: %{name}-core-doc < %{version}-%{release}
|
||||
Provides: %{name}-core-doc = %{version}-%{release}
|
||||
Obsoletes: %{name}-core-doc
|
||||
Provides: %{name}-core-doc
|
||||
|
||||
%description help
|
||||
%{summary}.
|
||||
@ -220,9 +208,9 @@ sed -i -e '1s@#!\( */usr/bin/env python\|%{__python2}\)$@#!%{__python3}@' \
|
||||
%make_build -C contrib/diff-highlight/
|
||||
|
||||
%install
|
||||
%make_install %{_smp_mflags} install-doc
|
||||
%make_install %{_smp_mflags} -C contrib/subtree/ install-doc
|
||||
%make_install %{_smp_mflags} -C contrib/contacts/ install-doc
|
||||
%make_install install-doc
|
||||
%make_install -C contrib/subtree/ install-doc
|
||||
%make_install -C contrib/contacts/ install-doc
|
||||
|
||||
install -p -m 644 README.md %{buildroot}%{_pkgdocdir}
|
||||
install -p -m 644 gitweb/INSTALL %{buildroot}%{_pkgdocdir}/INSTALL.gitweb
|
||||
@ -270,7 +258,7 @@ sed -i "/Git\/SVN/ d" perl-git-files
|
||||
# Split core files
|
||||
not_core_re="git-(add--interactive|contacts|credential-netrc|filter-branch|instaweb|request-pull|send-mail)|gitweb"
|
||||
grep -vE "$not_core_re|%{_mandir}" bin-man-doc-files > bin-files-core
|
||||
grep -E "$not_core_re" bin-man-doc-files > git-bin-files
|
||||
|
||||
|
||||
%check
|
||||
make %{?_smp_mflags} test
|
||||
@ -286,24 +274,21 @@ make %{?_smp_mflags} test
|
||||
|
||||
%files -f git-bin-files
|
||||
%defattr(-,root,root)
|
||||
%{_datadir}/git-core/templates/hooks/fsmonitor-watchman.sample
|
||||
%{_datadir}/git-core/templates/hooks/pre-rebase.sample
|
||||
%{_datadir}/git-core/templates/hooks/prepare-commit-msg.sample
|
||||
%{gitexecdir}/git-archimport
|
||||
%{gitexecdir}/git-credential-libsecret
|
||||
%{gitexecdir}/git-p4
|
||||
%{gitexecdir}/git-subtree
|
||||
%{gitexecdir}/mergetools/p4merge
|
||||
%{_datadir}/git-core/
|
||||
%{_datadir}/bash-completion/completions
|
||||
%doc README.md
|
||||
%license LGPL-2.1 COPYING
|
||||
|
||||
%files core -f bin-files-core
|
||||
%license LGPL-2.1 COPYING
|
||||
# exclude is best way here because of troubles with symlinks inside git-core/
|
||||
%exclude %{_datadir}/git-core/contrib/diff-highlight
|
||||
%exclude %{_datadir}/git-core/contrib/hooks/multimail
|
||||
%exclude %{_datadir}/git-core/contrib/hooks/update-paranoid
|
||||
%exclude %{_datadir}/git-core/contrib/hooks/setgitperms.perl
|
||||
%exclude %{_datadir}/git-core/templates/hooks/fsmonitor-watchman.sample
|
||||
%exclude %{_datadir}/git-core/templates/hooks/pre-rebase.sample
|
||||
%exclude %{_datadir}/git-core/templates/hooks/prepare-commit-msg.sample
|
||||
%{_datadir}/locale/{bg,ca,de,el,es,fr,id,is,it,ko,pl,pt_PT,ru,sv,tr,vi,zh_CN,zh_TW}/LC_MESSAGES/git.mo
|
||||
%{_datadir}/bash-completion/completions
|
||||
%{_datadir}/git-core/
|
||||
|
||||
@ -341,7 +326,6 @@ make %{?_smp_mflags} test
|
||||
%{gitexecdir}/*email*
|
||||
|
||||
%files -n perl-Git -f perl-git-files
|
||||
%{_mandir}/man3/Git.*
|
||||
|
||||
%files -n perl-Git-SVN -f perl-git-svn-files
|
||||
|
||||
@ -350,43 +334,11 @@ make %{?_smp_mflags} test
|
||||
%exclude %{_pkgdocdir}/{README.md,*.gitweb}
|
||||
%{_pkgdocdir}/*
|
||||
%{_mandir}/man1/git*.1.*
|
||||
%{_mandir}/man3/Git.*
|
||||
%{_mandir}/man5/git*.5.*
|
||||
%{_mandir}/man7/git*.7.*
|
||||
|
||||
%changelog
|
||||
* Mon May 20 2024 fuanan <fuanan3@h-partners.com> - 2.33.0-15
|
||||
- Type:CVE
|
||||
- ID:CVE-2024-32002
|
||||
- SUG:NA
|
||||
- DESC:Fix CVE-2024-32002
|
||||
|
||||
* Thu May 16 2024 fuanan <fuanan3@h-partners.com> - 2.33.0-14
|
||||
- Type:CVE
|
||||
- ID:CVE-2024-32021 CVE-2024-32004 CVE-2024-32020 CVE-2024-32465
|
||||
- SUG:NA
|
||||
- DESC:Fix CVE-2024-32021 CVE-2024-32004 CVE-2024-32020 CVE-2024-32465
|
||||
|
||||
* Fri Jul 7 2023 huyubiao <huyubiao@huawei.com> - 2.33.0-13
|
||||
- Type:bugfix
|
||||
- ID:NA
|
||||
- SUG:NA
|
||||
- DESC:Specifying Obsoletes and Provides version numbers, prevent upgrade conflicts.
|
||||
Delete git-core-doc from git, git-core-doc is provided by git-help.
|
||||
|
||||
* Wed May 17 2023 fuanan <fuanan3@h-partners.com> - 2.33.0-12
|
||||
- Type:bugfix
|
||||
- ID:NA
|
||||
- SUG:NA
|
||||
- DESC:Fix duplicate files in git primary package and subpackage
|
||||
|
||||
* Thu Apr 27 2023 fuanan <fuanan3@h-partners.com> - 2.33.0-11
|
||||
- Type:bugfix
|
||||
- ID:NA
|
||||
- SUG:NA
|
||||
- DESC:enable multithreading compilation and installation,
|
||||
move Git.3pm.gz to perl-Git,avoid installation conflicts,
|
||||
change BuildRequires from pcre-devel to pcre2-devel.
|
||||
|
||||
* Wed Apr 26 2023 fuanan <fuanan3@h-partners.com> - 2.33.0-10
|
||||
- Type:CVE
|
||||
- ID:CVE-2023-25652 CVE-2023-29007 CVE-2023-25815
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user