diff options
| author | Johannes Schindelin <johannes.schindelin@gmx.de> | 2024-04-12 09:45:28 +0200 |
|---|---|---|
| committer | Johannes Schindelin <johannes.schindelin@gmx.de> | 2024-04-19 12:38:37 +0200 |
| commit | 93a88f42db7ed9a975768df0e5f4516317c50dda (patch) | |
| tree | 425bd4d9bc74c6a172a6d29e28c468158f094a96 /builtin | |
| parent | 6741e917def23aff497ace35aa34debd1782330a (diff) | |
| parent | 47b6d90e91835082010da926f6a844d4441c57a6 (diff) | |
| download | git-93a88f42db7ed9a975768df0e5f4516317c50dda.tar.gz | |
Sync with 2.39.4
* maint-2.39: (38 commits)
Git 2.39.4
fsck: warn about symlink pointing inside a gitdir
core.hooksPath: add some protection while cloning
init.templateDir: consider this config setting protected
clone: prevent hooks from running during a clone
Add a helper function to compare file contents
init: refactor the template directory discovery into its own function
find_hook(): refactor the `STRIP_EXTENSION` logic
clone: when symbolic links collide with directories, keep the latter
entry: report more colliding paths
t5510: verify that D/F confusion cannot lead to an RCE
submodule: require the submodule path to contain directories only
clone_submodule: avoid using `access()` on directories
submodules: submodule paths must not contain symlinks
clone: prevent clashing git dirs when cloning submodule in parallel
t7423: add tests for symlinked submodule directories
has_dir_name(): do not get confused by characters < '/'
docs: document security issues around untrusted .git dirs
upload-pack: disable lazy-fetching by default
fetch/clone: detect dubious ownership of local repositories
...
Diffstat (limited to 'builtin')
| -rw-r--r-- | builtin/clone.c | 51 | ||||
| -rw-r--r-- | builtin/init-db.c | 22 | ||||
| -rw-r--r-- | builtin/submodule--helper.c | 86 | ||||
| -rw-r--r-- | builtin/upload-pack.c | 2 |
4 files changed, 135 insertions, 26 deletions
diff --git a/builtin/clone.c b/builtin/clone.c index 65b5b7db6d..5fa2901400 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -320,7 +320,20 @@ 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; + + /* + * 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); @@ -358,9 +371,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) { - strbuf_realpath(&realpath, src->buf, 1); - if (!link(realpath.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; @@ -373,8 +404,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) @@ -909,6 +938,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix) int err = 0, complete_refs_before_fetch = 1; int submodule_progress; int filter_submodules = 0; + const char *template_dir; + char *template_dir_dup = NULL; struct transport_ls_refs_options transport_ls_refs_options = TRANSPORT_LS_REFS_OPTIONS_INIT; @@ -928,6 +959,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix) usage_msg_opt(_("You must specify a repository to clone."), builtin_clone_usage, builtin_clone_options); + xsetenv("GIT_CLONE_PROTECTION_ACTIVE", "true", 0 /* allow user override */); + template_dir = get_template_dir(option_template); + if (*template_dir && !is_absolute_path(template_dir)) + template_dir = template_dir_dup = + absolute_pathdup(template_dir); + xsetenv("GIT_CLONE_TEMPLATE_DIR", template_dir, 1); + if (option_depth || option_since || option_not.nr) deepen = 1; if (option_single_branch == -1) @@ -1075,7 +1113,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } } - init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL, + init_db(git_dir, real_git_dir, template_dir, GIT_HASH_UNKNOWN, NULL, INIT_DB_QUIET); if (real_git_dir) { @@ -1419,6 +1457,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) free(dir); free(path); free(repo_to_free); + free(template_dir_dup); junk_mode = JUNK_LEAVE_ALL; transport_ls_refs_options_release(&transport_ls_refs_options); diff --git a/builtin/init-db.c b/builtin/init-db.c index dcaaf102ea..a101e7f94c 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -11,10 +11,6 @@ #include "parse-options.h" #include "worktree.h" -#ifndef DEFAULT_GIT_TEMPLATE_DIR -#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates" -#endif - #ifdef NO_TRUSTABLE_FILEMODE #define TEST_FILEMODE 0 #else @@ -93,8 +89,9 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path, } } -static void copy_templates(const char *template_dir, const char *init_template_dir) +static void copy_templates(const char *option_template) { + const char *template_dir = get_template_dir(option_template); struct strbuf path = STRBUF_INIT; struct strbuf template_path = STRBUF_INIT; size_t template_len; @@ -103,16 +100,8 @@ static void copy_templates(const char *template_dir, const char *init_template_d DIR *dir; char *to_free = NULL; - if (!template_dir) - template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT); - if (!template_dir) - template_dir = init_template_dir; - if (!template_dir) - template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR); - if (!template_dir[0]) { - free(to_free); + if (!template_dir || !*template_dir) return; - } strbuf_addstr(&template_path, template_dir); strbuf_complete(&template_path, '/'); @@ -200,7 +189,6 @@ static int create_default_files(const char *template_path, int reinit; int filemode; struct strbuf err = STRBUF_INIT; - const char *init_template_dir = NULL; const char *work_tree = get_git_work_tree(); /* @@ -212,9 +200,7 @@ static int create_default_files(const char *template_path, * values (since we've just potentially changed what's available on * disk). */ - git_config_get_pathname("init.templatedir", &init_template_dir); - copy_templates(template_path, init_template_dir); - free((char *)init_template_dir); + copy_templates(template_path); git_config_clear(); reset_shared_repository(); git_config(git_default_config, NULL); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 4c173d8b37..74a23759f7 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -294,6 +294,9 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, struct child_process cp = CHILD_PROCESS_INIT; char *displaypath; + if (validate_submodule_path(path) < 0) + exit(128); + displaypath = get_submodule_displaypath(path, info->prefix, info->super_prefix); @@ -626,6 +629,9 @@ static void status_submodule(const char *path, const struct object_id *ce_oid, .free_removed_argv_elements = 1, }; + if (validate_submodule_path(path) < 0) + exit(128); + if (!submodule_from_path(the_repository, null_oid(), path)) die(_("no submodule mapping found in .gitmodules for path '%s'"), path); @@ -1230,6 +1236,9 @@ static void sync_submodule(const char *path, const char *prefix, if (!is_submodule_active(the_repository, path)) return; + if (validate_submodule_path(path) < 0) + exit(128); + sub = submodule_from_path(the_repository, null_oid(), path); if (sub && sub->url) { @@ -1373,6 +1382,9 @@ static void deinit_submodule(const char *path, const char *prefix, struct strbuf sb_config = STRBUF_INIT; char *sub_git_dir = xstrfmt("%s/.git", path); + if (validate_submodule_path(path) < 0) + exit(128); + sub = submodule_from_path(the_repository, null_oid(), path); if (!sub || !sub->name) @@ -1654,16 +1666,42 @@ static char *clone_submodule_sm_gitdir(const char *name) return sm_gitdir; } +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; char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name); char *sm_alternate = NULL, *error_strategy = NULL; + struct stat st; struct child_process cp = CHILD_PROCESS_INIT; const char *clone_data_path = clone_data->path; char *to_free = NULL; + if (validate_submodule_path(clone_data_path) < 0) + exit(128); + if (!is_absolute_path(clone_data->path)) clone_data_path = to_free = xstrfmt("%s/%s", get_git_work_tree(), clone_data->path); @@ -1673,6 +1711,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); @@ -1717,10 +1759,18 @@ 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 { char *path; - if (clone_data->require_init && !access(clone_data_path, X_OK) && + 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(clone_data_path) < 0) @@ -1730,6 +1780,23 @@ static int clone_submodule(const struct module_clone_data *clone_data, free(path); } + /* + * We already performed this check at the beginning of this function, + * before cloning the objects. This tries to detect racy behavior e.g. + * in parallel clones, where another process could easily have made the + * gitdir nested _after_ it was created. + * + * To prevent further harm coming from this unintentionally-nested + * gitdir, let's disable it by deleting the `HEAD` file. + */ + if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0) { + char *head = xstrfmt("%s/HEAD", sm_gitdir); + unlink(head); + free(head); + die(_("refusing to create/use '%s' in another submodule's " + "git dir"), sm_gitdir); + } + connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0); p = git_pathdup_submodule(clone_data_path, "config"); @@ -2505,6 +2572,9 @@ static int update_submodule(struct update_data *update_data) { int ret; + if (validate_submodule_path(update_data->sm_path) < 0) + return -1; + ret = determine_submodule_update_strategy(the_repository, update_data->just_cloned, update_data->sm_path, @@ -2612,12 +2682,21 @@ static int update_submodules(struct update_data *update_data) for (i = 0; i < suc.update_clone_nr; i++) { struct update_clone_data ucd = suc.update_clone[i]; - int code; + int code = 128; oidcpy(&update_data->oid, &ucd.oid); update_data->just_cloned = ucd.just_cloned; update_data->sm_path = ucd.sub->path; + /* + * Verify that the submodule path does not contain any + * symlinks; if it does, it might have been tampered with. + * TODO: allow exempting it via + * `safe.submodule.path` or something + */ + if (validate_submodule_path(update_data->sm_path) < 0) + goto fail; + code = ensure_core_worktree(update_data->sm_path); if (code) goto fail; @@ -3329,6 +3408,9 @@ static int module_add(int argc, const char **argv, const char *prefix) normalize_path_copy(add_data.sm_path, add_data.sm_path); strip_dir_trailing_slashes(add_data.sm_path); + if (validate_submodule_path(add_data.sm_path) < 0) + exit(128); + die_on_index_match(add_data.sm_path, force); die_on_repo_without_commits(add_data.sm_path); diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c index 25b69da2bf..f446ff04f6 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); |
