diff --git a/CVE-2019-1348.patch b/CVE-2019-1348.patch new file mode 100644 index 0000000000000000000000000000000000000000..e1d9007bf5425242c3063f32b035af82817eafda --- /dev/null +++ b/CVE-2019-1348.patch @@ -0,0 +1,259 @@ +From 68061e3470210703cb15594194718d35094afdc0 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 29 Aug 2019 14:37:26 -0400 +Subject: [PATCH] fast-import: disallow "feature export-marks" by default + +The fast-import stream command "feature export-marks=" lets the +stream write marks to an arbitrary path. This may be surprising if you +are running fast-import against an untrusted input (which otherwise +cannot do anything except update Git objects and refs). + +Let's disallow the use of this feature by default, and provide a +command-line option to re-enable it (you can always just use the +command-line --export-marks as well, but the in-stream version provides +an easy way for exporters to control the process). + +This is a backwards-incompatible change, since the default is flipping +to the new, safer behavior. However, since the main users of the +in-stream versions would be import/export-based remote helpers, and +since we trust remote helpers already (which are already running +arbitrary code), we'll pass the new option by default when reading a +remote helper's stream. This should minimize the impact. + +Note that the implementation isn't totally simple, as we have to work +around the fact that fast-import doesn't parse its command-line options +until after it has read any "feature" lines from the stream. This is how +it lets command-line options override in-stream. But in our case, it's +important to parse the new --allow-unsafe-features first. + +There are three options for resolving this: + + 1. Do a separate "early" pass over the options. This is easy for us to + do because there are no command-line options that allow the + "unstuck" form (so there's no chance of us mistaking an argument + for an option), though it does introduce a risk of incorrect + parsing later (e.g,. if we convert to parse-options). + + 2. Move the option parsing phase back to the start of the program, but + teach the stream-reading code never to override an existing value. + This is tricky, because stream "feature" lines override each other + (meaning we'd have to start tracking the source for every option). + + 3. Accept that we might parse a "feature export-marks" line that is + forbidden, as long we don't _act_ on it until after we've parsed + the command line options. + + This would, in fact, work with the current code, but only because + the previous patch fixed the export-marks parser to avoid touching + the filesystem. + + So while it works, it does carry risk of somebody getting it wrong + in the future in a rather subtle and unsafe way. + +I've gone with option (1) here as simple, safe, and unlikely to cause +regressions. + +This fixes CVE-2019-1348. + +Signed-off-by: Jeff King +--- + Documentation/git-fast-import.txt | 14 ++++++++++++++ + fast-import.c | 25 +++++++++++++++++++++++++ + t/t9300-fast-import.sh | 23 +++++++++++++++-------- + transport-helper.c | 1 + + 4 files changed, 55 insertions(+), 8 deletions(-) + +diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt +index fad327a..76747ef 100644 +--- a/Documentation/git-fast-import.txt ++++ b/Documentation/git-fast-import.txt +@@ -51,6 +51,20 @@ OPTIONS + memory used by fast-import during this run. Showing this output + is currently the default, but can be disabled with --quiet. + ++--allow-unsafe-features:: ++ Many command-line options can be provided as part of the ++ fast-import stream itself by using the `feature` or `option` ++ commands. However, some of these options are unsafe (e.g., ++ allowing fast-import to access the filesystem outside of the ++ repository). These options are disabled by default, but can be ++ allowed by providing this option on the command line. This ++ currently impacts only the `feature export-marks` command. +++ ++ Only enable this option if you trust the program generating the ++ fast-import stream! This option is enabled automatically for ++ remote-helpers that use the `import` capability, as they are ++ already trusted to run their own code. ++ + Options for Frontends + ~~~~~~~~~~~~~~~~~~~~~ + +diff --git a/fast-import.c b/fast-import.c +index 71312d4..b001abc 100644 +--- a/fast-import.c ++++ b/fast-import.c +@@ -217,6 +217,7 @@ struct recent_command { + static struct strbuf new_data = STRBUF_INIT; + static int seen_data_command; + static int require_explicit_termination; ++static int allow_unsafe_features; + + /* Signal handling */ + static volatile sig_atomic_t checkpoint_requested; +@@ -3171,6 +3172,8 @@ static int parse_one_option(const char *option) + show_stats = 0; + } else if (!strcmp(option, "stats")) { + show_stats = 1; ++ } else if (!strcmp(option, "allow-unsafe-features")) { ++ ; /* already handled during early option parsing */ + } else { + return 0; + } +@@ -3178,6 +3181,13 @@ static int parse_one_option(const char *option) + return 1; + } + ++static void check_unsafe_feature(const char *feature, int from_stream) ++{ ++ if (from_stream && !allow_unsafe_features) ++ die(_("feature '%s' forbidden in input without --allow-unsafe-features"), ++ feature); ++} ++ + static int parse_one_feature(const char *feature, int from_stream) + { + const char *arg; +@@ -3189,6 +3199,7 @@ static int parse_one_feature(const char *feature, int from_stream) + } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { + option_import_marks(arg, from_stream, 1); + } else if (skip_prefix(feature, "export-marks=", &arg)) { ++ check_unsafe_feature(feature, from_stream); + option_export_marks(arg); + } else if (!strcmp(feature, "get-mark")) { + ; /* Don't die - this feature is supported */ +@@ -3315,6 +3326,20 @@ int cmd_main(int argc, const char **argv) + avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*)); + marks = mem_pool_calloc(&fi_mem_pool, 1, sizeof(struct mark_set)); + ++ /* ++ * We don't parse most options until after we've seen the set of ++ * "feature" lines at the start of the stream (which allows the command ++ * line to override stream data). But we must do an early parse of any ++ * command-line options that impact how we interpret the feature lines. ++ */ ++ for (i = 1; i < argc; i++) { ++ const char *arg = argv[i]; ++ if (*arg != '-' || !strcmp(arg, "--")) ++ break; ++ if (!strcmp(arg, "--allow-unsafe-features")) ++ allow_unsafe_features = 1; ++ } ++ + global_argc = argc; + global_argv = argv; + +diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh +index 913ff89..8ca58f5 100755 +--- a/t/t9300-fast-import.sh ++++ b/t/t9300-fast-import.sh +@@ -2115,6 +2115,11 @@ test_expect_success 'R: only one import-marks feature allowed per stream' ' + test_must_fail git fast-import input && ++ test_must_fail git fast-import input <<-EOF && + feature export-marks=git.marks +@@ -2125,7 +2130,7 @@ test_expect_success 'R: export-marks feature results in a marks file being creat + + EOF + +- git fast-import one.marks && + tail -n +3 marks.out > two.marks && +- git fast-import --import-marks=one.marks --import-marks=two.marks in = xdup(helper->out); + argv_array_push(&fastimport->args, "fast-import"); ++ argv_array_push(&fastimport->args, "--allow-unsafe-features"); + argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet"); + + if (data->bidi_import) { +-- +1.8.3.1 + diff --git a/CVE-2019-1349.patch b/CVE-2019-1349.patch new file mode 100644 index 0000000000000000000000000000000000000000..5e5f3558d375bc709ee69ab66f0e7b413a609f4c --- /dev/null +++ b/CVE-2019-1349.patch @@ -0,0 +1,237 @@ +From 629226a8902640470bb8cf9521f058340e7c0248 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Thu, 12 Sep 2019 14:20:39 +0200 +Subject: [PATCH 07/30] clone --recurse-submodules: prevent name squatting on + Windows + +In addition to preventing `.git` from being tracked by Git, on Windows +we also have to prevent `git~1` from being tracked, as the default NTFS +short name (also known as the "8.3 filename") for the file name `.git` +is `git~1`, otherwise it would be possible for malicious repositories to +write directly into the `.git/` directory, e.g. a `post-checkout` hook +that would then be executed _during_ a recursive clone. + +When we implemented appropriate protections in 2b4c6efc821 (read-cache: +optionally disallow NTFS .git variants, 2014-12-16), we had analyzed +carefully that the `.git` directory or file would be guaranteed to be +the first directory entry to be written. Otherwise it would be possible +e.g. for a file named `..git` to be assigned the short name `git~1` and +subsequently, the short name generated for `.git` would be `git~2`. Or +`git~3`. Or even `~9999999` (for a detailed explanation of the lengths +we have to go to protect `.gitmodules`, see the commit message of +e7cb0b4455c (is_ntfs_dotgit: match other .git files, 2018-05-11)). + +However, by exploiting two issues (that will be addressed in a related +patch series close by), it is currently possible to clone a submodule +into a non-empty directory: + +- On Windows, file names cannot end in a space or a period (for + historical reasons: the period separating the base name from the file + extension was not actually written to disk, and the base name/file + extension was space-padded to the full 8/3 characters, respectively). + Helpfully, when creating a directory under the name, say, `sub.`, that + trailing period is trimmed automatically and the actual name on disk + is `sub`. + + This means that while Git thinks that the submodule names `sub` and + `sub.` are different, they both access `.git/modules/sub/`. + +- While the backslash character is a valid file name character on Linux, + it is not so on Windows. As Git tries to be cross-platform, it + therefore allows backslash characters in the file names stored in tree + objects. + + Which means that it is totally possible that a submodule `c` sits next + to a file `c\..git`, and on Windows, during recursive clone a file + called `..git` will be written into `c/`, of course _before_ the + submodule is cloned. + +Note that the actual exploit is not quite as simple as having a +submodule `c` next to a file `c\..git`, as we have to make sure that the +directory `.git/modules/b` already exists when the submodule is checked +out, otherwise a different code path is taken in `module_clone()` that +does _not_ allow a non-empty submodule directory to exist already. + +Even if we will address both issues nearby (the next commit will +disallow backslash characters in tree entries' file names on Windows, +and another patch will disallow creating directories/files with trailing +spaces or periods), it is a wise idea to defend in depth against this +sort of attack vector: when submodules are cloned recursively, we now +_require_ the directory to be empty, addressing CVE-2019-1349. + +Note: the code path we patch is shared with the code path of `git +submodule update --init`, which must not expect, in general, that the +directory is empty. Hence we have to introduce the new option +`--force-init` and hand it all the way down from `git submodule` to the +actual `git submodule--helper` process that performs the initial clone. + +Reported-by: Nicolas Joly +Signed-off-by: Johannes Schindelin +--- + builtin/clone.c | 2 +- + builtin/submodule--helper.c | 14 ++++++++++++-- + git-submodule.sh | 6 ++++++ + t/t7415-submodule-names.sh | 31 +++++++++++++++++++++++++++++++ + 4 files changed, 50 insertions(+), 3 deletions(-) + +diff --git a/builtin/clone.c b/builtin/clone.c +index f665b28..e48476d 100644 +--- a/builtin/clone.c ++++ b/builtin/clone.c +@@ -790,7 +790,7 @@ static int checkout(int submodule_progress) + + if (!err && (option_recurse_submodules.nr > 0)) { + struct argv_array args = ARGV_ARRAY_INIT; +- argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL); ++ argv_array_pushl(&args, "submodule", "update", "--require-init", "--recursive", NULL); + + if (option_shallow_submodules == 1) + argv_array_push(&args, "--depth=1"); +diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c +index 909e77e..f3ba6aa 100644 +--- a/builtin/submodule--helper.c ++++ b/builtin/submodule--helper.c +@@ -19,6 +19,7 @@ + #include "diffcore.h" + #include "diff.h" + #include "object-store.h" ++#include "dir.h" + + #define OPT_QUIET (1 << 0) + #define OPT_CACHED (1 << 1) +@@ -1359,7 +1360,7 @@ static int module_clone(int argc, const char **argv, const char *prefix) + char *p, *path = NULL, *sm_gitdir; + struct strbuf sb = STRBUF_INIT; + struct string_list reference = STRING_LIST_INIT_NODUP; +- int dissociate = 0; ++ int dissociate = 0, require_init = 0; + char *sm_alternate = NULL, *error_strategy = NULL; + + struct option module_clone_options[] = { +@@ -1386,6 +1387,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) + OPT__QUIET(&quiet, "Suppress output for cloning a submodule"), + OPT_BOOL(0, "progress", &progress, + N_("force cloning progress")), ++ OPT_BOOL(0, "require-init", &require_init, ++ N_("disallow cloning into non-empty directory")), + OPT_END() + }; + +@@ -1424,6 +1427,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) + die(_("clone of '%s' into submodule path '%s' failed"), + url, path); + } else { ++ if (require_init && !access(path, X_OK) && !is_empty_dir(path)) ++ die(_("directory not empty: '%s'"), path); + if (safe_create_leading_directories_const(path) < 0) + die(_("could not create directory '%s'"), path); + strbuf_addf(&sb, "%s/index", sm_gitdir); +@@ -1536,6 +1541,7 @@ struct submodule_update_clone { + int recommend_shallow; + struct string_list references; + int dissociate; ++ unsigned require_init; + const char *depth; + const char *recursive_prefix; + const char *prefix; +@@ -1554,7 +1560,7 @@ struct submodule_update_clone { + int max_jobs; + }; + #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \ +- SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \ ++ SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, 0, \ + NULL, NULL, NULL, \ + NULL, 0, 0, 0, NULL, 0, 0, 1} + +@@ -1681,6 +1687,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, + argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL); + if (suc->recommend_shallow && sub->recommend_shallow == 1) + argv_array_push(&child->args, "--depth=1"); ++ if (suc->require_init) ++ argv_array_push(&child->args, "--require-init"); + argv_array_pushl(&child->args, "--path", sub->path, NULL); + argv_array_pushl(&child->args, "--name", sub->name, NULL); + argv_array_pushl(&child->args, "--url", url, NULL); +@@ -1870,6 +1878,8 @@ static int update_clone(int argc, const char **argv, const char *prefix) + OPT__QUIET(&suc.quiet, N_("don't print cloning progress")), + OPT_BOOL(0, "progress", &suc.progress, + N_("force cloning progress")), ++ OPT_BOOL(0, "require-init", &suc.require_init, ++ N_("disallow cloning into non-empty directory")), + OPT_END() + }; + +diff --git a/git-submodule.sh b/git-submodule.sh +index c7f58c5..58713c5 100755 +--- a/git-submodule.sh ++++ b/git-submodule.sh +@@ -36,6 +36,7 @@ reference= + cached= + recursive= + init= ++require_init= + files= + remote= + nofetch= +@@ -466,6 +467,10 @@ cmd_update() + -i|--init) + init=1 + ;; ++ --require-init) ++ init=1 ++ require_init=1 ++ ;; + --remote) + remote=1 + ;; +@@ -548,6 +553,7 @@ cmd_update() + ${reference:+"$reference"} \ + ${dissociate:+"--dissociate"} \ + ${depth:+--depth "$depth"} \ ++ ${require_init:+--require-init} \ + $recommend_shallow \ + $jobs \ + -- \ +diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh +index 49a37ef..75dcdff 100755 +--- a/t/t7415-submodule-names.sh ++++ b/t/t7415-submodule-names.sh +@@ -191,4 +191,35 @@ test_expect_success 'fsck detects corrupt .gitmodules' ' + ) + ' + ++test_expect_success MINGW 'prevent git~1 squatting on Windows' ' ++ git init squatting && ++ ( ++ cd squatting && ++ mkdir a && ++ touch a/..git && ++ git add a/..git && ++ test_tick && ++ git commit -m initial && ++ ++ modules="$(test_write_lines \ ++ "[submodule \"b.\"]" "url = ." "path = c" \ ++ "[submodule \"b\"]" "url = ." "path = d\\\\a" | ++ git hash-object -w --stdin)" && ++ rev="$(git rev-parse --verify HEAD)" && ++ hash="$(echo x | git hash-object -w --stdin)" && ++ git update-index --add \ ++ --cacheinfo 100644,$modules,.gitmodules \ ++ --cacheinfo 160000,$rev,c \ ++ --cacheinfo 160000,$rev,d\\a \ ++ --cacheinfo 100644,$hash,d./a/x \ ++ --cacheinfo 100644,$hash,d./a/..git && ++ test_tick && ++ git commit -m "module" ++ ) && ++ test_must_fail git \ ++ clone --recurse-submodules squatting squatting-clone 2>err && ++ test_i18ngrep "directory not empty" err && ++ ! grep gitdir squatting-clone/d/a/git~2 ++' ++ + test_done +-- +1.8.3.1 + diff --git a/CVE-2019-1350.patch b/CVE-2019-1350.patch new file mode 100644 index 0000000000000000000000000000000000000000..dccb4f87c75eae1094cf4bb4982b282f21a1e630 --- /dev/null +++ b/CVE-2019-1350.patch @@ -0,0 +1,99 @@ +From 6d8684161ee9c03bed5cb69ae76dfdddb85a0003 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Fri, 13 Sep 2019 16:32:43 +0200 +Subject: [PATCH] mingw: fix quoting of arguments + +We need to be careful to follow proper quoting rules. For example, if an +argument contains spaces, we have to quote them. Double-quotes need to +be escaped. Backslashes need to be escaped, but only if they are +followed by a double-quote character. + +We need to be _extra_ careful to consider the case where an argument +ends in a backslash _and_ needs to be quoted: in this case, we append a +double-quote character, i.e. the backslash now has to be escaped! + +The current code, however, fails to recognize that, and therefore can +turn an argument that ends in a single backslash into a quoted argument +that now ends in an escaped double-quote character. This allows +subsequent command-line parameters to be split and part of them being +mistaken for command-line options, e.g. through a maliciously-crafted +submodule URL during a recursive clone. + + +Technically, we would not need to quote _all_ arguments which end in a +backslash _unless_ the argument needs to be quoted anyway. For example, +`test\` would not need to be quoted, while `test \` would need to be. + +To keep the code simple, however, and therefore easier to reason about +and ensure its correctness, we now _always_ quote an argument that ends +in a backslash. + +This addresses CVE-2019-1350. + +Signed-off-by: Johannes Schindelin +--- + compat/mingw.c | 9 ++++++--- + t/t7416-submodule-dash-url.sh | 14 ++++++++++++++ + 2 files changed, 20 insertions(+), 3 deletions(-) + +diff --git a/compat/mingw.c b/compat/mingw.c +index 738f0a8..48cdf83 100644 +--- a/compat/mingw.c ++++ b/compat/mingw.c +@@ -1052,7 +1052,7 @@ static const char *quote_arg_msvc(const char *arg) + p++; + len++; + } +- if (*p == '"') ++ if (*p == '"' || !*p) + n += count*2 + 1; + continue; + } +@@ -1074,16 +1074,19 @@ static const char *quote_arg_msvc(const char *arg) + count++; + *d++ = *arg++; + } +- if (*arg == '"') { ++ if (*arg == '"' || !*arg) { + while (count-- > 0) + *d++ = '\\'; ++ /* don't escape the surrounding end quote */ ++ if (!*arg) ++ break; + *d++ = '\\'; + } + } + *d++ = *arg++; + } + *d++ = '"'; +- *d++ = 0; ++ *d++ = '\0'; + return q; + } + +diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh +index 1cd2c1c..5ba041f 100755 +--- a/t/t7416-submodule-dash-url.sh ++++ b/t/t7416-submodule-dash-url.sh +@@ -46,4 +46,18 @@ test_expect_success 'fsck rejects unprotected dash' ' + grep gitmodulesUrl err + ' + ++test_expect_success 'trailing backslash is handled correctly' ' ++ git init testmodule && ++ test_commit -C testmodule c && ++ git submodule add ./testmodule && ++ : ensure that the name ends in a double backslash && ++ sed -e "s|\\(submodule \"testmodule\\)\"|\\1\\\\\\\\\"|" \ ++ -e "s|url = .*|url = \" --should-not-be-an-option\"|" \ ++ <.gitmodules >.new && ++ mv .new .gitmodules && ++ git commit -am "Add testmodule" && ++ test_must_fail git clone --verbose --recurse-submodules . dolly 2>err && ++ test_i18ngrep ! "unknown option" err ++' ++ + test_done +-- +1.8.3.1 + diff --git a/CVE-2019-1351.patch b/CVE-2019-1351.patch new file mode 100644 index 0000000000000000000000000000000000000000..08eac8d52d796dce20d5ec41f91ad01655fa4db2 --- /dev/null +++ b/CVE-2019-1351.patch @@ -0,0 +1,135 @@ +From a94971b023f4583ea4b2c9c9f7a71d53f9781d58 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Fri, 6 Sep 2019 00:09:10 +0200 +Subject: [PATCH] mingw: handle `subst`-ed "DOS drives" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Over a decade ago, in 25fe217b86c (Windows: Treat Windows style path +names., 2008-03-05), Git was taught to handle absolute Windows paths, +i.e. paths that start with a drive letter and a colon. + +Unbeknownst to us, while drive letters of physical drives are limited to +letters of the English alphabet, there is a way to assign virtual drive +letters to arbitrary directories, via the `subst` command, which is +_not_ limited to English letters. + +It is therefore possible to have absolute Windows paths of the form +`1:\what\the\hex.txt`. Even "better": pretty much arbitrary Unicode +letters can also be used, e.g. `ä:\tschibät.sch`. + +While it can be sensibly argued that users who set up such funny drive +letters really seek adverse consequences, the Windows Operating System +is known to be a platform where many users are at the mercy of +administrators who have their very own idea of what constitutes a +reasonable setup. + +Therefore, let's just make sure that such funny paths are still +considered absolute paths by Git, on Windows. + +In addition to Unicode characters, pretty much any character is a valid +drive letter, as far as `subst` is concerned, even `:` and `"` or even a +space character. While it is probably the opposite of smart to use them, +let's safeguard `is_dos_drive_prefix()` against all of them. + +Note: `[::1]:repo` is a valid URL, but not a valid path on Windows. +As `[` is now considered a valid drive letter, we need to be very +careful to avoid misinterpreting such a string as valid local path in +`url_is_local_not_ssh()`. To do that, we use the just-introduced +function `is_valid_path()` (which will label the string as invalid file +name because of the colon characters). + +This fixes CVE-2019-1351. + +Reported-by: Nicolas Joly +Signed-off-by: Johannes Schindelin +--- + compat/win32/path-utils.c | 24 ++++++++++++++++++++++++ + compat/win32/path-utils.h | 4 ++-- + connect.c | 2 +- + t/t0060-path-utils.sh | 9 +++++++++ + 4 files changed, 36 insertions(+), 3 deletions(-) + +diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c +index d9d3641de8..70352af283 100644 +--- a/compat/win32/path-utils.c ++++ b/compat/win32/path-utils.c +@@ -1,5 +1,29 @@ + #include "../../git-compat-util.h" + ++int mingw_has_dos_drive_prefix(const char *path) ++{ ++ int i; ++ ++ /* ++ * Does it start with an ASCII letter (i.e. highest bit not set), ++ * followed by a colon? ++ */ ++ if (!(0x80 & (unsigned char)*path)) ++ return *path && path[1] == ':' ? 2 : 0; ++ ++ /* ++ * While drive letters must be letters of the English alphabet, it is ++ * possible to assign virtually _any_ Unicode character via `subst` as ++ * a drive letter to "virtual drives". Even `1`, or `ä`. Or fun stuff ++ * like this: ++ * ++ * subst ֍: %USERPROFILE%\Desktop ++ */ ++ for (i = 1; i < 4 && (0x80 & (unsigned char)path[i]); i++) ++ ; /* skip first UTF-8 character */ ++ return path[i] == ':' ? i + 1 : 0; ++} ++ + int win32_skip_dos_drive_prefix(char **path) + { + int ret = has_dos_drive_prefix(*path); +diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h +index 0f70d43920..22b0c63349 100644 +--- a/compat/win32/path-utils.h ++++ b/compat/win32/path-utils.h +@@ -1,5 +1,5 @@ +-#define has_dos_drive_prefix(path) \ +- (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0) ++int mingw_has_dos_drive_prefix(const char *path); ++#define has_dos_drive_prefix mingw_has_dos_drive_prefix + int win32_skip_dos_drive_prefix(char **path); + #define skip_dos_drive_prefix win32_skip_dos_drive_prefix + static inline int win32_is_dir_sep(int c) +diff --git a/connect.c b/connect.c +index 2778481264..4050a797bf 100644 +--- a/connect.c ++++ b/connect.c +@@ -511,7 +511,7 @@ int url_is_local_not_ssh(const char *url) + const char *colon = strchr(url, ':'); + const char *slash = strchr(url, '/'); + return !colon || (slash && slash < colon) || +- has_dos_drive_prefix(url); ++ (has_dos_drive_prefix(url) && is_valid_path(url)); + } + + static const char *prot_name(enum protocol protocol) +diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh +index c7b53e494b..e9b61efdff 100755 +--- a/t/t0060-path-utils.sh ++++ b/t/t0060-path-utils.sh +@@ -165,6 +165,15 @@ test_expect_success 'absolute path rejects the empty string' ' + test_must_fail test-tool path-utils absolute_path "" + ' + ++test_expect_success MINGW ':\\abc is an absolute path' ' ++ for letter in : \" C Z 1 ä ++ do ++ path=$letter:\\abc && ++ absolute="$(test-path-utils absolute_path "$path")" && ++ test "$path" = "$absolute" || return 1 ++ done ++' ++ + test_expect_success 'real path rejects the empty string' ' + test_must_fail test-tool path-utils real_path "" + ' +-- +2.23.0 + diff --git a/CVE-2019-1352.patch b/CVE-2019-1352.patch new file mode 100644 index 0000000000000000000000000000000000000000..a722068ff41699fd1b46561d4b7ad17494b9e6f3 --- /dev/null +++ b/CVE-2019-1352.patch @@ -0,0 +1,94 @@ +From 7c3745fc6185495d5765628b4dfe1bd2c25a2981 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Wed, 28 Aug 2019 12:22:17 +0200 +Subject: [PATCH] path: safeguard `.git` against NTFS Alternate Streams + Accesses + +Probably inspired by HFS' resource streams, NTFS supports "Alternate +Data Streams": by appending `:` to the file name, +information in addition to the file contents can be written and read, +information that is copied together with the file (unless copied to a +non-NTFS location). + +These Alternate Data Streams are typically used for things like marking +an executable as having just been downloaded from the internet (and +hence not necessarily being trustworthy). + +In addition to a stream name, a stream type can be appended, like so: +`::`. Unless specified, the default stream +type is `$DATA` for files and `$INDEX_ALLOCATION` for directories. In +other words, `.git::$INDEX_ALLOCATION` is a valid way to reference the +`.git` directory! + +In our work in Git v2.2.1 to protect Git on NTFS drives under +`core.protectNTFS`, we focused exclusively on NTFS short names, unaware +of the fact that NTFS Alternate Data Streams offer a similar attack +vector. + +Let's fix this. + +Seeing as it is better to be safe than sorry, we simply disallow paths +referring to *any* NTFS Alternate Data Stream of `.git`, not just +`::$INDEX_ALLOCATION`. This also simplifies the implementation. + +This closes CVE-2019-1352. + +Further reading about NTFS Alternate Data Streams: +https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/c54dec26-1551-4d3a-a0ea-4fa40f848eb3 + +Reported-by: Nicolas Joly +Signed-off-by: Johannes Schindelin +--- + path.c | 12 +++++++++++- + t/t1014-read-tree-confusing.sh | 1 + + 2 files changed, 12 insertions(+), 1 deletion(-) + +diff --git a/path.c b/path.c +index f62a37d..e39ecf4 100644 +--- a/path.c ++++ b/path.c +@@ -1321,10 +1321,19 @@ static int only_spaces_and_periods(const char *path, size_t len, size_t skip) + * `.git` is the first item in a directory, therefore it will be associated + * with the short name `git~1` (unless short names are disabled). + * ++ * - For yet other historical reasons, NTFS supports so-called "Alternate Data ++ * Streams", i.e. metadata associated with a given file, referred to via ++ * `::`. There exists a default stream ++ * type for directories, allowing `.git/` to be accessed via ++ * `.git::$INDEX_ALLOCATION/`. ++ * + * When this function returns 1, it indicates that the specified file/directory + * name refers to a `.git` file or directory, or to any of these synonyms, and + * Git should therefore not track it. + * ++ * For performance reasons, _all_ Alternate Data Streams of `.git/` are ++ * forbidden, not just `::$INDEX_ALLOCATION`. ++ * + * This function is intended to be used by `git fsck` even on platforms where + * the backslash is a regular filename character, therefore it needs to handle + * backlash characters in the provided `name` specially: they are interpreted +@@ -1335,7 +1344,8 @@ int is_ntfs_dotgit(const char *name) + size_t len; + + for (len = 0; ; len++) +- if (!name[len] || name[len] == '\\' || is_dir_sep(name[len])) { ++ if (!name[len] || name[len] == '\\' || is_dir_sep(name[len]) || ++ name[len] == ':') { + if (only_spaces_and_periods(name, len, 4) && + !strncasecmp(name, ".git", 4)) + return 1; +diff --git a/t/t1014-read-tree-confusing.sh b/t/t1014-read-tree-confusing.sh +index 2f5a25d..da3376b 100755 +--- a/t/t1014-read-tree-confusing.sh ++++ b/t/t1014-read-tree-confusing.sh +@@ -49,6 +49,7 @@ git~1 + .git.SPACE .git.{space} + .\\\\.GIT\\\\foobar backslashes + .git\\\\foobar backslashes2 ++.git...:alternate-stream + EOF + + test_expect_success 'utf-8 paths allowed with core.protectHFS off' ' +-- +1.8.3.1 + diff --git a/CVE-2019-1353.patch b/CVE-2019-1353.patch new file mode 100644 index 0000000000000000000000000000000000000000..53e7d506118715098221f047a2648728936db9fd --- /dev/null +++ b/CVE-2019-1353.patch @@ -0,0 +1,142 @@ +From 9102f958ee5254b10c0be72672aa3305bf4f4704 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Mon, 9 Sep 2019 21:04:41 +0200 +Subject: [PATCH] protect_ntfs: turn on NTFS protection by default + +Back in the DOS days, in the FAT file system, file names always +consisted of a base name of length 8 plus a file extension of length 3. +Shorter file names were simply padded with spaces to the full 8.3 +format. + +Later, the FAT file system was taught to support _also_ longer names, +with an 8.3 "short name" as primary file name. While at it, the same +facility allowed formerly illegal file names, such as `.git` (empty base +names were not allowed), which would have the "short name" `git~1` +associated with it. + +For backwards-compatibility, NTFS supports alternative 8.3 short +filenames, too, even if starting with Windows Vista, they are only +generated on the system drive by default. + +We addressed the problem that the `.git/` directory can _also_ be +accessed via `git~1/` (when short names are enabled) in 2b4c6efc821 +(read-cache: optionally disallow NTFS .git variants, 2014-12-16), i.e. +since Git v1.9.5, by introducing the config setting `core.protectNTFS` +and enabling it by default on Windows. + +In the meantime, Windows 10 introduced the "Windows Subsystem for Linux" +(short: WSL), i.e. a way to run Linux applications/distributions in a +thinly-isolated subsystem on Windows (giving rise to many a "2016 is the +Year of Linux on the Desktop" jokes). WSL is getting increasingly +popular, also due to the painless way Linux application can operate +directly ("natively") on files on Windows' file system: the Windows +drives are mounted automatically (e.g. `C:` as `/mnt/c/`). + +Taken together, this means that we now have to enable the safe-guards of +Git v1.9.5 also in WSL: it is possible to access a `.git` directory +inside `/mnt/c/` via the 8.3 name `git~1` (unless short name generation +was disabled manually). Since regular Linux distributions run in WSL, +this means we have to enable `core.protectNTFS` at least on Linux, too. + +To enable Services for Macintosh in Windows NT to store so-called +resource forks, NTFS introduced "Alternate Data Streams". Essentially, +these constitute additional metadata that are connected to (and copied +with) their associated files, and they are accessed via pseudo file +names of the form `filename::`. + +In a recent patch, we extended `core.protectNTFS` to also protect +against accesses via NTFS Alternate Data Streams, e.g. to prevent +contents of the `.git/` directory to be "tracked" via yet another +alternative file name. + +While it is not possible (at least by default) to access files via NTFS +Alternate Data Streams from within WSL, the defaults on macOS when +mounting network shares via SMB _do_ allow accessing files and +directories in that way. Therefore, we need to enable `core.protectNTFS` +on macOS by default, too, and really, on any Operating System that can +mount network shares via SMB/CIFS. + +A couple of approaches were considered for fixing this: + +1. We could perform a dynamic NTFS check similar to the `core.symlinks` + check in `init`/`clone`: instead of trying to create a symbolic link + in the `.git/` directory, we could create a test file and try to + access `.git/config` via 8.3 name and/or Alternate Data Stream. + +2. We could simply "flip the switch" on `core.protectNTFS`, to make it + "on by default". + +The obvious downside of 1. is that it won't protect worktrees that were +clone with a vulnerable Git version already. We considered patching code +paths that check out files to check whether we're running on an NTFS +system dynamically and persist the result in the repository-local config +setting `core.protectNTFS`, but in the end decided that this solution +would be too fragile, and too involved. + +The obvious downside of 2. is that everybody will have to "suffer" the +performance penalty incurred from calling `is_ntfs_dotgit()` on every +path, even in setups where. + +After the recent work to accelerate `is_ntfs_dotgit()` in most cases, +it looks as if the time spent on validating ten million random +file names increases only negligibly (less than 20ms, well within the +standard deviation of ~50ms). Therefore the benefits outweigh the cost. + +Another downside of this is that paths that might have been acceptable +previously now will be forbidden. Realistically, though, this is an +improvement because public Git hosters already would reject any `git +push` that contains such file names. + +Note: There might be a similar problem mounting HFS+ on Linux. However, +this scenario has been considered unlikely and in light of the cost (in +the aforementioned benchmark, `core.protectHFS = true` increased the +time from ~440ms to ~610ms), it was decided _not_ to touch the default +of `core.protectHFS`. + +This change addresses CVE-2019-1353. + +Reported-by: Nicolas Joly +Helped-by: Garima Singh +Signed-off-by: Johannes Schindelin +--- + config.mak.uname | 3 +-- + environment.c | 2 +- + 2 files changed, 2 insertions(+), 3 deletions(-) + +diff --git a/config.mak.uname b/config.mak.uname +index db7f06b..3e46a8e 100644 +--- a/config.mak.uname ++++ b/config.mak.uname +@@ -454,7 +454,6 @@ ifneq ($(USE_MSVC_CRTDBG),) + # Optionally enable memory leak reporting. + BASIC_CFLAGS += -DUSE_MSVC_CRTDBG + endif +- BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1 + # Always give "-Zi" to the compiler and "-debug" to linker (even in + # release mode) to force a PDB to be generated (like RelWithDebInfo). + BASIC_CFLAGS += -Zi +@@ -616,7 +615,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) + compat/win32/path-utils.o \ + compat/win32/pthread.o compat/win32/syslog.o \ + compat/win32/dirent.o +- BASIC_CFLAGS += -DWIN32 -DPROTECT_NTFS_DEFAULT=1 ++ BASIC_CFLAGS += -DWIN32 + EXTLIBS += -lws2_32 + GITLIBS += git.res + PTHREAD_LIBS = +diff --git a/environment.c b/environment.c +index 89af47c..9f5f381 100644 +--- a/environment.c ++++ b/environment.c +@@ -80,7 +80,7 @@ + int protect_hfs = PROTECT_HFS_DEFAULT; + + #ifndef PROTECT_NTFS_DEFAULT +-#define PROTECT_NTFS_DEFAULT 0 ++#define PROTECT_NTFS_DEFAULT 1 + #endif + int protect_ntfs = PROTECT_NTFS_DEFAULT; + const char *core_fsmonitor; +-- +1.8.3.1 + diff --git a/CVE-2019-1354.patch b/CVE-2019-1354.patch new file mode 100644 index 0000000000000000000000000000000000000000..e8c1efb729532ec23170f00970d5b37c2b7f18a5 --- /dev/null +++ b/CVE-2019-1354.patch @@ -0,0 +1,105 @@ +From e1d911dd4c7b76a5a8cec0f5c8de15981e34da83 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Thu, 12 Sep 2019 14:54:05 +0200 +Subject: [PATCH] mingw: disallow backslash characters in tree objects' file + names + +The backslash character is not a valid part of a file name on Windows. +Hence it is dangerous to allow writing files that were unpacked from +tree objects, when the stored file name contains a backslash character: +it will be misinterpreted as directory separator. + +This not only causes ambiguity when a tree contains a blob `a\b` and a +tree `a` that contains a blob `b`, but it also can be used as part of an +attack vector to side-step the careful protections against writing into +the `.git/` directory during a clone of a maliciously-crafted +repository. + +Let's prevent that, addressing CVE-2019-1354. + +Note: we guard against backslash characters in tree objects' file names +_only_ on Windows (because on other platforms, even on those where NTFS +volumes can be mounted, the backslash character is _not_ a directory +separator), and _only_ when `core.protectNTFS = true` (because users +might need to generate tree objects for other platforms, of course +without touching the worktree, e.g. using `git update-index +--cacheinfo`). + +Signed-off-by: Johannes Schindelin +--- + t/t1450-fsck.sh | 1 + + t/t7415-submodule-names.sh | 8 +++++--- + t/t9350-fast-export.sh | 1 + + tree-walk.c | 6 ++++++ + 4 files changed, 13 insertions(+), 3 deletions(-) + +diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh +index cb4b66e..33c955f 100755 +--- a/t/t1450-fsck.sh ++++ b/t/t1450-fsck.sh +@@ -419,6 +419,7 @@ while read name path pretty; do + ( + git init $name-$type && + cd $name-$type && ++ git config core.protectNTFS false && + echo content >file && + git add file && + git commit -m base && +diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh +index e1cd0a3..7c65e7a 100755 +--- a/t/t7415-submodule-names.sh ++++ b/t/t7415-submodule-names.sh +@@ -89,16 +89,18 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' ' + git hash-object -w --stdin)" && + rev="$(git rev-parse --verify HEAD)" && + hash="$(echo x | git hash-object -w --stdin)" && +- git update-index --add \ ++ git -c core.protectNTFS=false update-index --add \ + --cacheinfo 100644,$modules,.gitmodules \ + --cacheinfo 160000,$rev,c \ + --cacheinfo 160000,$rev,d\\a \ + --cacheinfo 100644,$hash,d./a/x \ + --cacheinfo 100644,$hash,d./a/..git && + test_tick && +- git commit -m "module" ++ git -c core.protectNTFS=false commit -m "module" && ++ test_must_fail git show HEAD: 2>err && ++ test_i18ngrep backslash err + ) && +- test_must_fail git \ ++ test_must_fail git -c core.protectNTFS=false \ + clone --recurse-submodules squatting squatting-clone 2>err && + test_i18ngrep "directory not empty" err && + ! grep gitdir squatting-clone/d/a/git~2 +diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh +index 866ddf6..e606207 100755 +--- a/t/t9350-fast-export.sh ++++ b/t/t9350-fast-export.sh +@@ -421,6 +421,7 @@ test_expect_success 'directory becomes symlink' ' + + test_expect_success 'fast-export quotes pathnames' ' + git init crazy-paths && ++ test_config -C crazy-paths core.protectNTFS false && + (cd crazy-paths && + blob=$(echo foo | git hash-object -w --stdin) && + git update-index --add \ +diff --git a/tree-walk.c b/tree-walk.c +index d459fed..54ff959 100644 +--- a/tree-walk.c ++++ b/tree-walk.c +@@ -41,6 +41,12 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l + strbuf_addstr(err, _("empty filename in tree entry")); + return -1; + } ++#ifdef GIT_WINDOWS_NATIVE ++ if (protect_ntfs && strchr(path, '\\')) { ++ strbuf_addf(err, _("filename in tree entry contains backslash: '%s'"), path); ++ return -1; ++ } ++#endif + len = strlen(path) + 1; + + /* Initialize the descriptor entry */ +-- +1.8.3.1 + diff --git a/CVE-2019-1387.patch b/CVE-2019-1387.patch new file mode 100644 index 0000000000000000000000000000000000000000..38c882a21180b22e11d8f2b8154d9d0e3b1120d2 --- /dev/null +++ b/CVE-2019-1387.patch @@ -0,0 +1,181 @@ +From a8dee3ca610f5a1d403634492136c887f83b59d2 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Tue, 1 Oct 2019 23:27:18 +0200 +Subject: [PATCH] Disallow dubiously-nested submodule git directories + +Currently it is technically possible to let a submodule's git +directory point right into the git dir of a sibling submodule. + +Example: the git directories of two submodules with the names `hippo` +and `hippo/hooks` would be `.git/modules/hippo/` and +`.git/modules/hippo/hooks/`, respectively, but the latter is already +intended to house the former's hooks. + +In most cases, this is just confusing, but there is also a (quite +contrived) attack vector where Git can be fooled into mistaking remote +content for file contents it wrote itself during a recursive clone. + +Let's plug this bug. + +To do so, we introduce the new function `validate_submodule_git_dir()` +which simply verifies that no git dir exists for any leading directories +of the submodule name (if there are any). + +Note: this patch specifically continues to allow sibling modules names +of the form `core/lib`, `core/doc`, etc, as long as `core` is not a +submodule name. + +This fixes CVE-2019-1387. + +Reported-by: Nicolas Joly +Signed-off-by: Johannes Schindelin +--- + builtin/submodule--helper.c | 4 ++++ + submodule.c | 49 +++++++++++++++++++++++++++++++++++++++++++-- + submodule.h | 5 +++++ + t/t7415-submodule-names.sh | 23 +++++++++++++++++++++ + 4 files changed, 79 insertions(+), 2 deletions(-) + +diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c +index f3ba6aa..a083b99 100644 +--- a/builtin/submodule--helper.c ++++ b/builtin/submodule--helper.c +@@ -1416,6 +1416,10 @@ static int module_clone(int argc, const char **argv, const char *prefix) + } else + path = xstrdup(path); + ++ if (validate_submodule_git_dir(sm_gitdir, name) < 0) ++ die(_("refusing to create/use '%s' in another submodule's " ++ "git dir"), sm_gitdir); ++ + if (!file_exists(sm_gitdir)) { + if (safe_create_leading_directories_const(sm_gitdir) < 0) + die(_("could not create directory '%s'"), sm_gitdir); +diff --git a/submodule.c b/submodule.c +index 0f199c5..9da7181 100644 +--- a/submodule.c ++++ b/submodule.c +@@ -1993,6 +1993,47 @@ int submodule_move_head(const char *path, + return ret; + } + ++int validate_submodule_git_dir(char *git_dir, const char *submodule_name) ++{ ++ size_t len = strlen(git_dir), suffix_len = strlen(submodule_name); ++ char *p; ++ int ret = 0; ++ ++ if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' || ++ strcmp(p, submodule_name)) ++ BUG("submodule name '%s' not a suffix of git dir '%s'", ++ submodule_name, git_dir); ++ ++ /* ++ * We prevent the contents of sibling submodules' git directories to ++ * clash. ++ * ++ * Example: having a submodule named `hippo` and another one named ++ * `hippo/hooks` would result in the git directories ++ * `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively, ++ * but the latter directory is already designated to contain the hooks ++ * of the former. ++ */ ++ for (; *p; p++) { ++ if (is_dir_sep(*p)) { ++ char c = *p; ++ ++ *p = '\0'; ++ if (is_git_directory(git_dir)) ++ ret = -1; ++ *p = c; ++ ++ if (ret < 0) ++ return error(_("submodule git dir '%s' is " ++ "inside git dir '%.*s'"), ++ git_dir, ++ (int)(p - git_dir), git_dir); ++ } ++ } ++ ++ return 0; ++} ++ + /* + * Embeds a single submodules git directory into the superprojects git dir, + * non recursively. +@@ -2000,7 +2041,7 @@ int submodule_move_head(const char *path, + static void relocate_single_git_dir_into_superproject(const char *path) + { + char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL; +- const char *new_git_dir; ++ char *new_git_dir; + const struct submodule *sub; + + if (submodule_uses_worktrees(path)) +@@ -2018,10 +2059,14 @@ static void relocate_single_git_dir_into_superproject(const char *path) + if (!sub) + die(_("could not lookup name for submodule '%s'"), path); + +- new_git_dir = git_path("modules/%s", sub->name); ++ new_git_dir = git_pathdup("modules/%s", sub->name); ++ if (validate_submodule_git_dir(new_git_dir, sub->name) < 0) ++ die(_("refusing to move '%s' into an existing git dir"), ++ real_old_git_dir); + if (safe_create_leading_directories_const(new_git_dir) < 0) + die(_("could not create directory '%s'"), new_git_dir); + real_new_git_dir = real_pathdup(new_git_dir, 1); ++ free(new_git_dir); + + fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"), + get_super_prefix_or_empty(), path, +diff --git a/submodule.h b/submodule.h +index 8072e6d..c81ec1a 100644 +--- a/submodule.h ++++ b/submodule.h +@@ -124,6 +124,11 @@ int push_unpushed_submodules(struct repository *r, + */ + int submodule_to_gitdir(struct strbuf *buf, const char *submodule); + ++/* ++ * Make sure that no submodule's git dir is nested in a sibling submodule's. ++ */ ++int validate_submodule_git_dir(char *git_dir, const char *submodule_name); ++ + #define SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0) + #define SUBMODULE_MOVE_HEAD_FORCE (1<<1) + int submodule_move_head(const char *path, +diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh +index 3008ba8..a44578f 100755 +--- a/t/t7415-submodule-names.sh ++++ b/t/t7415-submodule-names.sh +@@ -224,4 +224,27 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' ' + ! grep gitdir squatting-clone/d/a/git~2 + ' + ++test_expect_success 'git dirs of sibling submodules must not be nested' ' ++ git init nested && ++ test_commit -C nested nested && ++ ( ++ cd nested && ++ cat >.gitmodules <<-EOF && ++ [submodule "hippo"] ++ url = . ++ path = thing1 ++ [submodule "hippo/hooks"] ++ url = . ++ path = thing2 ++ EOF ++ git clone . thing1 && ++ git clone . thing2 && ++ git add .gitmodules thing1 thing2 && ++ test_tick && ++ git commit -m nested ++ ) && ++ test_must_fail git clone --recurse-submodules nested clone 2>err && ++ test_i18ngrep "is inside git dir" err ++' ++ + test_done +-- +1.8.3.1 + diff --git a/CVE-2019-19604.patch b/CVE-2019-19604.patch new file mode 100644 index 0000000000000000000000000000000000000000..1d0161f2948a207b6acb1ec7a2c19e2afdc05d12 --- /dev/null +++ b/CVE-2019-19604.patch @@ -0,0 +1,148 @@ +From e904deb89d9a9669a76a426182506a084d3f6308 Mon Sep 17 00:00:00 2001 +From: Jonathan Nieder +Date: Thu, 5 Dec 2019 01:28:28 -0800 +Subject: [PATCH] submodule: reject submodule.update = !command in .gitmodules + +Since ac1fbbda2013 (submodule: do not copy unknown update mode from +.gitmodules, 2013-12-02), Git has been careful to avoid copying + + [submodule "foo"] + update = !run an arbitrary scary command + +from .gitmodules to a repository's local config, copying in the +setting 'update = none' instead. The gitmodules(5) manpage documents +the intention: + + The !command form is intentionally ignored here for security + reasons + +Unfortunately, starting with v2.20.0-rc0 (which integrated ee69b2a9 +(submodule--helper: introduce new update-module-mode helper, +2018-08-13, first released in v2.20.0-rc0)), there are scenarios where +we *don't* ignore it: if the config store contains no +submodule.foo.update setting, the submodule-config API falls back to +reading .gitmodules and the repository-supplied !command gets run +after all. + +This was part of a general change over time in submodule support to +read more directly from .gitmodules, since unlike .git/config it +allows a project to change values between branches and over time +(while still allowing .git/config to override things). But it was +never intended to apply to this kind of dangerous configuration. + +The behavior change was not advertised in ee69b2a9's commit message +and was missed in review. + +Let's take the opportunity to make the protection more robust, even in +Git versions that are technically not affected: instead of quietly +converting 'update = !command' to 'update = none', noisily treat it as +an error. Allowing the setting but treating it as meaning something +else was just confusing; users are better served by seeing the error +sooner. Forbidding the construct makes the semantics simpler and +means we can check for it in fsck (in a separate patch). + +As a result, the submodule-config API cannot read this value from +.gitmodules under any circumstance, and we can declare with confidence + + For security reasons, the '!command' form is not accepted + here. + +Reported-by: Joern Schneeweisz +Signed-off-by: Jonathan Nieder +Signed-off-by: Johannes Schindelin +--- + Documentation/gitmodules.txt | 5 ++--- + submodule-config.c | 10 +++++++++- + t/t7406-submodule-update.sh | 14 ++++++++------ + 3 files changed, 19 insertions(+), 10 deletions(-) + +diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt +index a66e95b..f260b55 100644 +--- a/Documentation/gitmodules.txt ++++ b/Documentation/gitmodules.txt +@@ -44,9 +44,8 @@ submodule..update:: + submodule init` to initialize the configuration variable of + the same name. Allowed values here are 'checkout', 'rebase', + 'merge' or 'none'. See description of 'update' command in +- linkgit:git-submodule[1] for their meaning. Note that the +- '!command' form is intentionally ignored here for security +- reasons. ++ linkgit:git-submodule[1] for their meaning. For security ++ reasons, the '!command' form is not accepted here. + + submodule..branch:: + A remote branch name for tracking updates in the upstream submodule. +diff --git a/submodule-config.c b/submodule-config.c +index 4264ee2..3acc6ad 100644 +--- a/submodule-config.c ++++ b/submodule-config.c +@@ -405,6 +405,13 @@ struct parse_config_parameter { + int overwrite; + }; + ++/* ++ * Parse a config item from .gitmodules. ++ * ++ * This does not handle submodule-related configuration from the main ++ * config store (.git/config, etc). Callers are responsible for ++ * checking for overrides in the main config store when appropriate. ++ */ + static int parse_config(const char *var, const char *value, void *data) + { + struct parse_config_parameter *me = data; +@@ -482,7 +489,8 @@ static int parse_config(const char *var, const char *value, void *data) + warn_multiple_config(me->treeish_name, submodule->name, + "update"); + else if (parse_submodule_update_strategy(value, +- &submodule->update_strategy) < 0) ++ &submodule->update_strategy) < 0 || ++ submodule->update_strategy.type == SM_UPDATE_COMMAND) + die(_("invalid value for %s"), var); + } else if (!strcmp(item.buf, "shallow")) { + if (!me->overwrite && submodule->recommend_shallow != -1) +diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh +index c973278..031ae85 100755 +--- a/t/t7406-submodule-update.sh ++++ b/t/t7406-submodule-update.sh +@@ -407,12 +407,12 @@ test_expect_success 'submodule update - command in .git/config' ' + ) + ' + +-test_expect_success 'submodule update - command in .gitmodules is ignored' ' ++test_expect_success 'submodule update - command in .gitmodules is rejected' ' + test_when_finished "git -C super reset --hard HEAD^" && + git -C super config -f .gitmodules submodule.submodule.update "!false" && + git -C super commit -a -m "add command to .gitmodules file" && + git -C super/submodule reset --hard $submodulesha1^ && +- git -C super submodule update submodule ++ test_must_fail git -C super submodule update submodule + ' + + cat << EOF >expect +@@ -481,6 +481,9 @@ test_expect_success 'recursive submodule update - command in .git/config catches + ' + + test_expect_success 'submodule init does not copy command into .git/config' ' ++ test_when_finished "git -C super update-index --force-remove submodule1" && ++ test_when_finished git config -f super/.gitmodules \ ++ --remove-section submodule.submodule1 && + (cd super && + git ls-files -s submodule >out && + H=$(cut -d" " -f2 out) && +@@ -489,10 +492,9 @@ test_expect_success 'submodule init does not copy command into .git/config' ' + git config -f .gitmodules submodule.submodule1.path submodule1 && + git config -f .gitmodules submodule.submodule1.url ../submodule && + git config -f .gitmodules submodule.submodule1.update !false && +- git submodule init submodule1 && +- echo "none" >expect && +- git config submodule.submodule1.update >actual && +- test_cmp expect actual ++ test_must_fail git submodule init submodule1 && ++ test_expect_code 1 git config submodule.submodule1.update >actual && ++ test_must_be_empty actual + ) + ' + +-- +1.8.3.1 + diff --git a/fast-import-delay-creating-leading-directories-for-e.patch b/fast-import-delay-creating-leading-directories-for-e.patch new file mode 100644 index 0000000000000000000000000000000000000000..d4b898d4d0c947fcdfb57fef98183f2138a6c845 --- /dev/null +++ b/fast-import-delay-creating-leading-directories-for-e.patch @@ -0,0 +1,88 @@ +From e5e484f5bb9690ac2aaab2d0ca23e7433b2ac8c0 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 29 Aug 2019 13:33:48 -0400 +Subject: [PATCH 05/30] fast-import: delay creating leading directories for + export-marks + +When we parse the --export-marks option, we don't immediately open the +file, but we do create any leading directories. This can be especially +confusing when a command-line option overrides an in-stream one, in +which case we'd create the leading directory for the in-stream file, +even though we never actually write the file. + +Let's instead create the directories just before opening the file, which +means we'll create only useful directories. Note that this could change +the handling of relative paths if we chdir() in between, but we don't +actually do so; the only permanent chdir is from setup_git_directory() +which runs before either code path (potentially we should take the +pre-setup dir into account to avoid surprising the user, but that's an +orthogonal change). + +The test just adapts the existing "override" test to use paths with +leading directories. This checks both that the correct directory is +created (which worked before but was not tested), and that the +overridden one is not (our new fix here). + +While we're here, let's also check the error result of +safe_create_leading_directories(). We'd presumably notice any failure +immediately after when we try to open the file itself, but we can give a +more specific error message in this case. + +Signed-off-by: Jeff King +--- + fast-import.c | 7 ++++++- + t/t9300-fast-import.sh | 13 +++++++++++-- + 2 files changed, 17 insertions(+), 3 deletions(-) + +diff --git a/fast-import.c b/fast-import.c +index b05d560d0a..92e84d28a4 100644 +--- a/fast-import.c ++++ b/fast-import.c +@@ -1826,6 +1826,12 @@ static void dump_marks(void) + if (!export_marks_file || (import_marks_file && !import_marks_file_done)) + return; + ++ if (safe_create_leading_directories_const(export_marks_file)) { ++ failure |= error_errno("unable to create leading directories of %s", ++ export_marks_file); ++ return; ++ } ++ + if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0) < 0) { + failure |= error_errno("Unable to write marks file %s", + export_marks_file); +@@ -3238,7 +3244,6 @@ static void option_active_branches(const char *branches) + static void option_export_marks(const char *marks) + { + export_marks_file = make_fast_import_path(marks); +- safe_create_leading_directories_const(export_marks_file); + } + + static void option_cat_blob_fd(const char *fd) +diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh +index f79fcf4c1e..f5f3e2cb71 100755 +--- a/t/t9300-fast-import.sh ++++ b/t/t9300-fast-import.sh +@@ -2132,8 +2132,17 @@ test_expect_success 'R: export-marks feature results in a marks file being creat + ' + + test_expect_success 'R: export-marks options can be overridden by commandline options' ' +- git fast-import --export-marks=other.marks input <<-\EOF && ++ feature export-marks=feature-sub/git.marks ++ blob ++ mark :1 ++ data 3 ++ hi ++ ++ EOF ++ git fast-import --export-marks=cmdline-sub/other.marks +Date: Thu, 29 Aug 2019 13:07:04 -0400 +Subject: [PATCH 04/30] fast-import: stop creating leading directories for + import-marks + +When asked to import marks from "subdir/file.marks", we create the +leading directory "subdir" if it doesn't exist. This makes no sense for +importing marks, where we only ever open the path for reading. + +Most of the time this would be a noop, since if the marks file exists, +then the leading directories exist, too. But if it doesn't (e.g., +because --import-marks-if-exists was used), then we'd create the useless +directory. + +This dates back to 580d5f83e7 (fast-import: always create marks_file +directories, 2010-03-29). Even then it was useless, so it seems to have +been added in error alongside the --export-marks case (which _is_ +helpful). + +Signed-off-by: Jeff King +--- + fast-import.c | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/fast-import.c b/fast-import.c +index 30b9479a75..b05d560d0a 100644 +--- a/fast-import.c ++++ b/fast-import.c +@@ -3198,7 +3198,6 @@ static void option_import_marks(const char *marks, + } + + import_marks_file = make_fast_import_path(marks); +- safe_create_leading_directories_const(import_marks_file); + import_marks_file_from_stream = from_stream; + import_marks_file_ignore_missing = ignore_missing; + } +-- +2.17.1 + diff --git a/fast-import-tighten-parsing-of-boolean-command-line-.patch b/fast-import-tighten-parsing-of-boolean-command-line-.patch new file mode 100644 index 0000000000000000000000000000000000000000..63955b523d3adb7455e15aecbd5886f9639e84f3 --- /dev/null +++ b/fast-import-tighten-parsing-of-boolean-command-line-.patch @@ -0,0 +1,39 @@ +From a4d15d0cfb630844eb48a11f69ccaef9fa8719f4 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 29 Aug 2019 11:25:45 -0400 +Subject: [PATCH 03/30] fast-import: tighten parsing of boolean command line + options + +We parse options like "--max-pack-size=" using skip_prefix(), which +makes sense to get at the bytes after the "=". However, we also parse +"--quiet" and "--stats" with skip_prefix(), which allows things like +"--quiet-nonsense" to behave like "--quiet". + +This was a mistaken conversion in 0f6927c229 (fast-import: put option +parsing code in separate functions, 2009-12-04). Let's tighten this to +an exact match, which was the original intent. + +Signed-off-by: Jeff King +--- + fast-import.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/fast-import.c b/fast-import.c +index 69886687ce..30b9479a75 100644 +--- a/fast-import.c ++++ b/fast-import.c +@@ -3282,9 +3282,9 @@ static int parse_one_option(const char *option) + option_active_branches(option); + } else if (skip_prefix(option, "export-pack-edges=", &option)) { + option_export_pack_edges(option); +- } else if (starts_with(option, "quiet")) { ++ } else if (!strcmp(option, "quiet")) { + show_stats = 0; +- } else if (starts_with(option, "stats")) { ++ } else if (!strcmp(option, "stats")) { + show_stats = 1; + } else { + return 0; +-- +2.17.1 + diff --git a/git.spec b/git.spec index 93d384b5bfba47dd0eaf008c668ed31f60178f47..8f58af9ff01518b809209eec02345df2341aa714 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.23.0 -Release: 8 +Release: 9 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -12,13 +12,27 @@ Source100: git-gui.desktop Source101: git@.service.in Source102: git.socket +Patch0: t9300-drop-some-useless-uses-of-cat.patch +Patch1: fast-import-tighten-parsing-of-boolean-command-line-.patch +Patch2: fast-import-stop-creating-leading-directories-for-im.patch +Patch3: fast-import-delay-creating-leading-directories-for-e.patch +Patch4: CVE-2019-1348.patch +Patch5: CVE-2019-1349.patch +Patch6: CVE-2019-1350.patch +Patch7: CVE-2019-1351.patch +Patch8: path.c-document-the-purpose-of-is_ntfs_dotgit.patch +Patch9: CVE-2019-1352.patch +Patch10: CVE-2019-1353.patch +Patch11: CVE-2019-1354.patch +Patch12: CVE-2019-1387.patch +Patch13: CVE-2019-19604.patch + 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: less zlib openssh-clients perl(Term::ReadKey) perl-Git Obsoletes: %{name}-core %{name}-core-doc %{name}-subtree %{name}-p4 Provides: %{name} <= %{version}-%{release} %{name}-core %{name}-subtree %{name}-p4 - %description Git is a free and open source distributed version control system designed to handle everything from small to very large projects @@ -117,7 +131,7 @@ Provides: %{name}-core-doc %{summary}. %prep -%autosetup -n %{name}-%{version} +%autosetup -n %{name}-%{version} -p1 rm -rf perl/Git/LoadCPAN{.pm,/} grep -rlZ '^use Git::LoadCPAN::' | xargs -r0 sed -i 's/Git::LoadCPAN:://g' @@ -265,6 +279,11 @@ make test %{_mandir}/man7/git*.7.* %changelog +* Mon Feb 03 2020 openEuler Buildteam - 2.23.0-9 +- fix CVE-2019-1348 CVE-2019-1349 CVE-2019-1350 CVE-2019-1351 CVE-2019-1352 + CVE-2019-1353 CVE-2019-1354 CVE-2019-1387 CVE-2019-19604 + fix test error + * Thu Jan 09 2020 openEuler Buildteam - 2.23.0-8 - Delete unneeded files diff --git a/path.c-document-the-purpose-of-is_ntfs_dotgit.patch b/path.c-document-the-purpose-of-is_ntfs_dotgit.patch new file mode 100644 index 0000000000000000000000000000000000000000..1abe2e83d6a6d07da82afd193d84e2a2b74c73fc --- /dev/null +++ b/path.c-document-the-purpose-of-is_ntfs_dotgit.patch @@ -0,0 +1,56 @@ +From 525e7fba7854c23ee3530d0bf88d75f106f14c95 Mon Sep 17 00:00:00 2001 +From: Johannes Schindelin +Date: Mon, 16 Sep 2019 20:44:31 +0200 +Subject: [PATCH] path.c: document the purpose of `is_ntfs_dotgit()` + +Previously, this function was completely undocumented. It is worth, +though, to explain what is going on, as it is not really obvious at all. + +Suggested-by: Garima Singh +Signed-off-by: Johannes Schindelin +--- + path.c | 28 ++++++++++++++++++++++++++++ + 1 file changed, 28 insertions(+) + +diff --git a/path.c b/path.c +index 9ac0531..22bd0b6 100644 +--- a/path.c ++++ b/path.c +@@ -1302,6 +1302,34 @@ static int only_spaces_and_periods(const char *path, size_t len, size_t skip) + return 1; + } + ++/* ++ * On NTFS, we need to be careful to disallow certain synonyms of the `.git/` ++ * directory: ++ * ++ * - For historical reasons, file names that end in spaces or periods are ++ * automatically trimmed. Therefore, `.git . . ./` is a valid way to refer ++ * to `.git/`. ++ * ++ * - For other historical reasons, file names that do not conform to the 8.3 ++ * format (up to eight characters for the basename, three for the file ++ * extension, certain characters not allowed such as `+`, etc) are associated ++ * with a so-called "short name", at least on the `C:` drive by default. ++ * Which means that `git~1/` is a valid way to refer to `.git/`. ++ * ++ * Note: Technically, `.git/` could receive the short name `git~2` if the ++ * short name `git~1` were already used. In Git, however, we guarantee that ++ * `.git` is the first item in a directory, therefore it will be associated ++ * with the short name `git~1` (unless short names are disabled). ++ * ++ * When this function returns 1, it indicates that the specified file/directory ++ * name refers to a `.git` file or directory, or to any of these synonyms, and ++ * Git should therefore not track it. ++ * ++ * This function is intended to be used by `git fsck` even on platforms where ++ * the backslash is a regular filename character, therefore it needs to handle ++ * backlash characters in the provided `name` specially: they are interpreted ++ * as directory separators. ++ */ + int is_ntfs_dotgit(const char *name) + { + size_t len; +-- +1.8.3.1 + diff --git a/t9300-drop-some-useless-uses-of-cat.patch b/t9300-drop-some-useless-uses-of-cat.patch new file mode 100644 index 0000000000000000000000000000000000000000..994fad201718bcdb410724363cfd332fbc70a514 --- /dev/null +++ b/t9300-drop-some-useless-uses-of-cat.patch @@ -0,0 +1,61 @@ +From f94804c1f2626831c6bdf8cc269a571324e3f2f2 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Thu, 29 Aug 2019 11:19:18 -0400 +Subject: [PATCH] t9300: drop some useless uses of cat + +These waste a process, and make the line longer than it needs to be. + +Signed-off-by: Jeff King +--- + t/t9300-fast-import.sh | 10 +++++----- + 1 file changed, 5 insertions(+), 5 deletions(-) + +diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh +index d47560b..1d2a751 100755 +--- a/t/t9300-fast-import.sh ++++ b/t/t9300-fast-import.sh +@@ -2125,12 +2125,12 @@ test_expect_success 'R: export-marks feature results in a marks file being creat + + EOF + +- cat input | git fast-import && ++ git fast-import output && ++ git fast-import 2>output