From 68061e3470210703cb15594194718d35094afdc0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 29 Aug 2019 14:37:26 -0400 Subject: 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 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'Documentation') diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 3d3d219e58..fbb3f914f2 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -50,6 +50,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 ~~~~~~~~~~~~~~~~~~~~~ -- cgit v1.3 From a52ed76142f6e8d993bb4c50938a408966eb2b7c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 29 Aug 2019 15:08:42 -0400 Subject: fast-import: disallow "feature import-marks" by default As with export-marks in the previous commit, import-marks can access the filesystem. This is significantly less dangerous than export-marks because it only involves reading from arbitrary paths, rather than writing them. However, it could still be surprising and have security implications (e.g., exfiltrating data from a service that accepts fast-import streams). Let's lump it (and its "if-exists" counterpart) in with export-marks, and enable the in-stream version only if --allow-unsafe-features is set. Signed-off-by: Jeff King --- Documentation/git-fast-import.txt | 3 ++- fast-import.c | 2 ++ t/t9300-fast-import.sh | 22 +++++++++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-) (limited to 'Documentation') diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index fbb3f914f2..ff71fc2962 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -57,7 +57,8 @@ OPTIONS 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. + currently impacts only the `export-marks`, `import-marks`, and + `import-marks-if-exists` feature commands. + Only enable this option if you trust the program generating the fast-import stream! This option is enabled automatically for diff --git a/fast-import.c b/fast-import.c index 967077ad0b..93c3838254 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3344,8 +3344,10 @@ static int parse_one_feature(const char *feature, int from_stream) if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { + check_unsafe_feature("import-marks", from_stream); option_import_marks(arg, from_stream, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { + check_unsafe_feature("import-marks-if-exists", from_stream); option_import_marks(arg, from_stream, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { check_unsafe_feature(feature, from_stream); diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index ba5a35c32c..77104f9daa 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2106,6 +2106,14 @@ test_expect_success 'R: abort on receiving feature after data command' ' test_must_fail git fast-import git.marks && + echo "feature import-marks=git.marks" >input && + test_must_fail git fast-import input && + test_must_fail git fast-import git.marks && >git2.marks && @@ -2114,7 +2122,7 @@ test_expect_success 'R: only one import-marks feature allowed per stream' ' feature import-marks=git2.marks EOF - test_must_fail git fast-import expect && - git fast-import --export-marks=io.marks <<-\EOF && + git fast-import --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=not_io.marks EOF test_cmp expect io.marks && @@ -2221,7 +2230,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' echo ":1 $blob" >expect && echo ":2 $blob" >>expect && - git fast-import --export-marks=io.marks <<-\EOF && + git fast-import --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=io.marks blob mark :2 @@ -2234,7 +2244,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' echo ":3 $blob" >>expect && git fast-import --import-marks=io.marks \ - --export-marks=io.marks <<-\EOF && + --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=not_io.marks blob mark :3 @@ -2247,7 +2258,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' >expect && git fast-import --import-marks-if-exists=not_io.marks \ - --export-marks=io.marks <<-\EOF && + --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=io.marks EOF test_cmp expect io.marks -- cgit v1.3 From 66d2a6159f511924e7e0b8a21c93538879bfd622 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Dec 2019 19:58:46 +0100 Subject: Git 2.14.6 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.14.6.txt | 54 +++++++++++++++++++++++++++++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.14.6.txt (limited to 'Documentation') diff --git a/Documentation/RelNotes/2.14.6.txt b/Documentation/RelNotes/2.14.6.txt new file mode 100644 index 0000000000..72b7af6799 --- /dev/null +++ b/Documentation/RelNotes/2.14.6.txt @@ -0,0 +1,54 @@ +Git v2.14.6 Release Notes +========================= + +This release addresses the security issues CVE-2019-1348, +CVE-2019-1349, CVE-2019-1350, CVE-2019-1351, CVE-2019-1352, +CVE-2019-1353, CVE-2019-1354, and CVE-2019-1387. + +Fixes since v2.14.5 +------------------- + + * CVE-2019-1348: + The --export-marks option of git fast-import is exposed also via + the in-stream command feature export-marks=... and it allows + overwriting arbitrary paths. + + * CVE-2019-1349: + When submodules are cloned recursively, under certain circumstances + Git could be fooled into using the same Git directory twice. We now + require the directory to be empty. + + * CVE-2019-1350: + Incorrect quoting of command-line arguments allowed remote code + execution during a recursive clone in conjunction with SSH URLs. + + * CVE-2019-1351: + While the only permitted drive letters for physical drives on + Windows are letters of the US-English alphabet, this restriction + does not apply to virtual drives assigned via subst : + . Git mistook such paths for relative paths, allowing writing + outside of the worktree while cloning. + + * CVE-2019-1352: + Git was unaware of NTFS Alternate Data Streams, allowing files + inside the .git/ directory to be overwritten during a clone. + + * CVE-2019-1353: + When running Git in the Windows Subsystem for Linux (also known as + "WSL") while accessing a working directory on a regular Windows + drive, none of the NTFS protections were active. + + * CVE-2019-1354: + Filenames on Linux/Unix can contain backslashes. On Windows, + backslashes are directory separators. Git did not use to refuse to + write out tracked files with such filenames. + + * CVE-2019-1387: + Recursive clones are currently affected by a vulnerability that is + caused by too-lax validation of submodule names, allowing very + targeted attacks via remote code execution in recursive clones. + +Credit for finding these vulnerabilities goes to Microsoft Security +Response Center, in particular to Nicolas Joly. The `fast-import` +fixes were provided by Jeff King, the other fixes by Johannes +Schindelin with help from Garima Singh. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 40680482ce..46557afb1e 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.14.5 +DEF_VER=v2.14.6 LF=' ' diff --git a/RelNotes b/RelNotes index a127ce63f2..229381cd97 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.14.5.txt \ No newline at end of file +Documentation/RelNotes/2.14.6.txt \ No newline at end of file -- cgit v1.3 From e904deb89d9a9669a76a426182506a084d3f6308 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Dec 2019 01:28:28 -0800 Subject: 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 | 12 ++++++++++-- t/t7406-submodule-update.sh | 14 ++++++++------ 3 files changed, 20 insertions(+), 11 deletions(-) (limited to 'Documentation') diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index db5d47eb19..ac44a1510c 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 3414fa1c1b..464908df76 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -396,6 +396,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; @@ -473,8 +480,9 @@ 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) - die(_("invalid value for %s"), var); + &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) warn_multiple_config(me->treeish_name, submodule->name, diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 6f083c4d68..779932457a 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -406,12 +406,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 @@ -480,6 +480,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 && H=$(git ls-files -s submodule | cut -d" " -f2) && mkdir submodule1 && @@ -487,10 +490,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 ) ' -- cgit v1.3 From 7cdafcaacf677b9e0700fa988c247bda192db48d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Dec 2019 21:33:29 +0100 Subject: Git 2.15.4 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.15.4.txt | 11 +++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.15.4.txt (limited to 'Documentation') diff --git a/Documentation/RelNotes/2.15.4.txt b/Documentation/RelNotes/2.15.4.txt new file mode 100644 index 0000000000..dc241cba34 --- /dev/null +++ b/Documentation/RelNotes/2.15.4.txt @@ -0,0 +1,11 @@ +Git v2.15.4 Release Notes +========================= + +This release merges up the fixes that appear in v2.14.6 to address +the security issues CVE-2019-1348, CVE-2019-1349, CVE-2019-1350, +CVE-2019-1351, CVE-2019-1352, CVE-2019-1353, CVE-2019-1354, and +CVE-2019-1387; see the release notes for that version for details. + +In conjunction with a vulnerability that was fixed in v2.20.2, +`.gitmodules` is no longer allowed to contain entries of the form +`submodule..update=!command`. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 4a63ce35ad..6fe1c9dcc7 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.15.3 +DEF_VER=v2.15.4 LF=' ' diff --git a/RelNotes b/RelNotes index e7fe59f5d0..03f405050e 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.15.3.txt \ No newline at end of file +Documentation/RelNotes/2.15.4.txt \ No newline at end of file -- cgit v1.3 From eb288bc455ac67e3ceeff90daf6f25972bb586d0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Dec 2019 21:45:07 +0100 Subject: Git 2.16.6 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.16.6.txt | 8 ++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.16.6.txt (limited to 'Documentation') diff --git a/Documentation/RelNotes/2.16.6.txt b/Documentation/RelNotes/2.16.6.txt new file mode 100644 index 0000000000..438306e60b --- /dev/null +++ b/Documentation/RelNotes/2.16.6.txt @@ -0,0 +1,8 @@ +Git v2.16.6 Release Notes +========================= + +This release merges up the fixes that appear in v2.14.6 and in +v2.15.4 addressing the security issues CVE-2019-1348, CVE-2019-1349, +CVE-2019-1350, CVE-2019-1351, CVE-2019-1352, CVE-2019-1353, +CVE-2019-1354, and CVE-2019-1387; see the release notes for those +versions for details. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 64f5097bcb..3c4ff15b48 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.16.5 +DEF_VER=v2.16.6 LF=' ' diff --git a/RelNotes b/RelNotes index 7b0f25d4c7..232b7f1a89 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.16.5.txt \ No newline at end of file +Documentation/RelNotes/2.16.6.txt \ No newline at end of file -- cgit v1.3 From a5ab8d03173458b76b8452efd90a7173f490c132 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 4 Dec 2019 22:13:04 +0100 Subject: Git 2.17.3 Signed-off-by: Johannes Schindelin --- Documentation/RelNotes/2.17.3.txt | 12 ++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.17.3.txt (limited to 'Documentation') diff --git a/Documentation/RelNotes/2.17.3.txt b/Documentation/RelNotes/2.17.3.txt new file mode 100644 index 0000000000..5a46c94271 --- /dev/null +++ b/Documentation/RelNotes/2.17.3.txt @@ -0,0 +1,12 @@ +Git v2.17.3 Release Notes +========================= + +This release merges up the fixes that appear in v2.14.6 and in +v2.15.4 addressing the security issues CVE-2019-1348, CVE-2019-1349, +CVE-2019-1350, CVE-2019-1351, CVE-2019-1352, CVE-2019-1353, +CVE-2019-1354, and CVE-2019-1387; see the release notes for those +versions for details. + +In addition, `git fsck` was taught to identify `.gitmodules` entries +of the form `submodule..update=!command`, which have been +disallowed in v2.15.4. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index bc54879938..bd6cd16e3d 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.17.2 +DEF_VER=v2.17.3 LF=' ' diff --git a/RelNotes b/RelNotes index 733d1745a9..d14bdb5eda 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.17.2.txt \ No newline at end of file +Documentation/RelNotes/2.17.3.txt \ No newline at end of file -- cgit v1.3