Merge pull request !7 from syyhao/dev
This commit is contained in:
openeuler-ci-bot 2020-02-04 12:37:24 +08:00 committed by Gitee
commit 55a952a593
15 changed files with 1706 additions and 3 deletions

259
CVE-2019-1348.patch Normal file
View File

@ -0,0 +1,259 @@
From 68061e3470210703cb15594194718d35094afdc0 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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=<path>" 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 <peff@peff.net>
---
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_expect_success 'R: export-marks feature forbidden by default' '
+ echo "feature export-marks=git.marks" >input &&
+ test_must_fail git fast-import <input
+'
+
test_expect_success 'R: export-marks feature results in a marks file being created' '
cat >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 <input &&
+ git fast-import --allow-unsafe-features <input &&
grep :1 git.marks
'
@@ -2138,7 +2143,8 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
hi
EOF
- git fast-import --export-marks=cmdline-sub/other.marks <input &&
+ git fast-import --allow-unsafe-features \
+ --export-marks=cmdline-sub/other.marks <input &&
grep :1 cmdline-sub/other.marks &&
test_path_is_missing feature-sub
'
@@ -2146,7 +2152,7 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
test_expect_success 'R: catch typo in marks file name' '
test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
echo "feature import-marks=nonexistent.marks" |
- test_must_fail git fast-import
+ test_must_fail git fast-import --allow-unsafe-features
'
test_expect_success 'R: import and output marks can be the same file' '
@@ -2248,7 +2254,7 @@ test_expect_success 'R: import to output marks works without any content' '
feature export-marks=marks.new
EOF
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.out marks.new
'
@@ -2258,7 +2264,7 @@ test_expect_success 'R: import marks prefers commandline marks file over the str
feature export-marks=marks.new
EOF
- git fast-import --import-marks=marks.out <input &&
+ git fast-import --import-marks=marks.out --allow-unsafe-features <input &&
test_cmp marks.out marks.new
'
@@ -2271,7 +2277,8 @@ test_expect_success 'R: multiple --import-marks= should be honoured' '
head -n2 marks.out > one.marks &&
tail -n +3 marks.out > two.marks &&
- git fast-import --import-marks=one.marks --import-marks=two.marks <input &&
+ git fast-import --import-marks=one.marks --import-marks=two.marks \
+ --allow-unsafe-features <input &&
test_cmp marks.out combined.marks
'
@@ -2284,7 +2291,7 @@ test_expect_success 'R: feature relative-marks should be honoured' '
mkdir -p .git/info/fast-import/ &&
cp marks.new .git/info/fast-import/relative.in &&
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.new .git/info/fast-import/relative.out
'
@@ -2296,7 +2303,7 @@ test_expect_success 'R: feature no-relative-marks should be honoured' '
feature export-marks=non-relative.out
EOF
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.new non-relative.out
'
diff --git a/transport-helper.c b/transport-helper.c
index 6b05a88..f828a6d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -425,6 +425,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti
child_process_init(fastimport);
fastimport->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

237
CVE-2019-1349.patch Normal file
View File

@ -0,0 +1,237 @@
From 629226a8902640470bb8cf9521f058340e7c0248 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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

99
CVE-2019-1350.patch Normal file
View File

@ -0,0 +1,99 @@
From 6d8684161ee9c03bed5cb69ae76dfdddb85a0003 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
---
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

135
CVE-2019-1351.patch Normal file
View File

@ -0,0 +1,135 @@
From a94971b023f4583ea4b2c9c9f7a71d53f9781d58 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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 '<drive-letter>:\\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

94
CVE-2019-1352.patch Normal file
View File

@ -0,0 +1,94 @@
From 7c3745fc6185495d5765628b4dfe1bd2c25a2981 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 `:<stream-name>` 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:
`:<stream-name>:<stream-type>`. 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 <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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
+ * `<filename>:<stream-name>:<stream-type>`. 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

142
CVE-2019-1353.patch Normal file
View File

@ -0,0 +1,142 @@
From 9102f958ee5254b10c0be72672aa3305bf4f4704 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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:<stream-name>:<stream-type>`.
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 <Nicolas.Joly@microsoft.com>
Helped-by: Garima Singh <garima.singh@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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

105
CVE-2019-1354.patch Normal file
View File

@ -0,0 +1,105 @@
From e1d911dd4c7b76a5a8cec0f5c8de15981e34da83 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
---
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

181
CVE-2019-1387.patch Normal file
View File

@ -0,0 +1,181 @@
From a8dee3ca610f5a1d403634492136c887f83b59d2 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <Nicolas.Joly@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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

148
CVE-2019-19604.patch Normal file
View File

@ -0,0 +1,148 @@
From e904deb89d9a9669a76a426182506a084d3f6308 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
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 <jschneeweisz@gitlab.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
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.<name>.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.<name>.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

View File

@ -0,0 +1,88 @@
From e5e484f5bb9690ac2aaab2d0ca23e7433b2ac8c0 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
---
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 &&
- grep :1 other.marks
+ cat >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 <input &&
+ grep :1 cmdline-sub/other.marks &&
+ test_path_is_missing feature-sub
'
test_expect_success 'R: catch typo in marks file name' '
--
2.17.1

View File

@ -0,0 +1,40 @@
From 9d09df88970ab91a08de15f0b2f9ab9b77ffd2cd Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
---
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

View File

@ -0,0 +1,39 @@
From a4d15d0cfb630844eb48a11f69ccaef9fa8719f4 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
---
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

View File

@ -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 <buildteam@openeuler.org> - 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 <buildteam@openeuler.org> - 2.23.0-8
- Delete unneeded files

View File

@ -0,0 +1,56 @@
From 525e7fba7854c23ee3530d0bf88d75f106f14c95 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <garima.singh@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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

View File

@ -0,0 +1,61 @@
From f94804c1f2626831c6bdf8cc269a571324e3f2f2 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
---
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 <input &&
grep :1 git.marks
'
test_expect_success 'R: export-marks options can be overridden by commandline options' '
- cat input | git fast-import --export-marks=other.marks &&
+ git fast-import --export-marks=other.marks <input &&
grep :1 other.marks
'
@@ -2242,7 +2242,7 @@ test_expect_success 'R: import to output marks works without any content' '
feature export-marks=marks.new
EOF
- cat input | git fast-import &&
+ git fast-import <input &&
test_cmp marks.out marks.new
'
@@ -2252,7 +2252,7 @@ test_expect_success 'R: import marks prefers commandline marks file over the str
feature export-marks=marks.new
EOF
- cat input | git fast-import --import-marks=marks.out &&
+ git fast-import --import-marks=marks.out <input &&
test_cmp marks.out marks.new
'
@@ -2560,7 +2560,7 @@ test_expect_success 'R: quiet option results in no stats being output' '
EOF
- cat input | git fast-import 2> output &&
+ git fast-import 2>output <input &&
test_must_be_empty output
'
--
1.8.3.1