diff --git a/backport-CVE-2022-39253-builtin-clone.c-disallow-local-clones-with-symlinks.patch b/backport-CVE-2022-39253-builtin-clone.c-disallow-local-clones-with-symlinks.patch new file mode 100644 index 0000000000000000000000000000000000000000..0fc724b3f5bf5b79150467154786faff3594f025 --- /dev/null +++ b/backport-CVE-2022-39253-builtin-clone.c-disallow-local-clones-with-symlinks.patch @@ -0,0 +1,194 @@ +From 6f054f9fb3a501c35b55c65e547a244f14c38d56 Mon Sep 17 00:00:00 2001 +From: Taylor Blau +Date: Thu, 28 Jul 2022 17:35:17 -0400 +Subject: [PATCH] builtin/clone.c: disallow `--local` clones with symlinks + +When cloning a repository with `--local`, Git relies on either making a +hardlink or copy to every file in the "objects" directory of the source +repository. This is done through the callpath `cmd_clone()` -> +`clone_local()` -> `copy_or_link_directory()`. + +The way this optimization works is by enumerating every file and +directory recursively in the source repository's `$GIT_DIR/objects` +directory, and then either making a copy or hardlink of each file. The +only exception to this rule is when copying the "alternates" file, in +which case paths are rewritten to be absolute before writing a new +"alternates" file in the destination repo. + +One quirk of this implementation is that it dereferences symlinks when +cloning. This behavior was most recently modified in 36596fd2df (clone: +better handle symlinked files at .git/objects/, 2019-07-10), which +attempted to support `--local` clones of repositories with symlinks in +their objects directory in a platform-independent way. + +Unfortunately, this behavior of dereferencing symlinks (that is, +creating a hardlink or copy of the source's link target in the +destination repository) can be used as a component in attacking a +victim by inadvertently exposing the contents of file stored outside of +the repository. + +Take, for example, a repository that stores a Dockerfile and is used to +build Docker images. When building an image, Docker copies the directory +contents into the VM, and then instructs the VM to execute the +Dockerfile at the root of the copied directory. This protects against +directory traversal attacks by copying symbolic links as-is without +dereferencing them. + +That is, if a user has a symlink pointing at their private key material +(where the symlink is present in the same directory as the Dockerfile, +but the key itself is present outside of that directory), the key is +unreadable to a Docker image, since the link will appear broken from the +container's point of view. + +This behavior enables an attack whereby a victim is convinced to clone a +repository containing an embedded submodule (with a URL like +"file:///proc/self/cwd/path/to/submodule") which has a symlink pointing +at a path containing sensitive information on the victim's machine. If a +user is tricked into doing this, the contents at the destination of +those symbolic links are exposed to the Docker image at runtime. + +One approach to preventing this behavior is to recreate symlinks in the +destination repository. But this is problematic, since symlinking the +objects directory are not well-supported. (One potential problem is that +when sharing, e.g. a "pack" directory via symlinks, different writers +performing garbage collection may consider different sets of objects to +be reachable, enabling a situation whereby garbage collecting one +repository may remove reachable objects in another repository). + +Instead, prohibit the local clone optimization when any symlinks are +present in the `$GIT_DIR/objects` directory of the source repository. +Users may clone the repository again by prepending the "file://" scheme +to their clone URL, or by adding the `--no-local` option to their `git +clone` invocation. + +The directory iterator used by `copy_or_link_directory()` must no longer +dereference symlinks (i.e., it *must* call `lstat()` instead of `stat()` +in order to discover whether or not there are symlinks present). This has +no bearing on the overall behavior, since we will immediately `die()` on +encounter a symlink. + +Note that t5604.33 suggests that we do support local clones with +symbolic links in the source repository's objects directory, but this +was likely unintentional, or at least did not take into consideration +the problem with sharing parts of the objects directory with symbolic +links at the time. Update this test to reflect which options are and +aren't supported. + +Helped-by: Johannes Schindelin +Signed-off-by: Taylor Blau +--- + builtin/clone.c | 8 +++--- + t/t5604-clone-reference.sh | 50 ++++++++++++++------------------------ + 2 files changed, 23 insertions(+), 35 deletions(-) + +diff --git a/builtin/clone.c b/builtin/clone.c +index e335734b4c..e626073b1f 100644 +--- a/builtin/clone.c ++++ b/builtin/clone.c +@@ -420,13 +420,11 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + int src_len, dest_len; + struct dir_iterator *iter; + int iter_status; +- unsigned int flags; + struct strbuf realpath = STRBUF_INIT; + + mkdir_if_missing(dest->buf, 0777); + +- flags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_FOLLOW_SYMLINKS; +- iter = dir_iterator_begin(src->buf, flags); ++ iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC); + + if (!iter) + die_errno(_("failed to start iterator over '%s'"), src->buf); +@@ -442,6 +440,10 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, + strbuf_setlen(dest, dest_len); + strbuf_addstr(dest, iter->relative_path); + ++ if (S_ISLNK(iter->st.st_mode)) ++ die(_("symlink '%s' exists, refusing to clone with --local"), ++ iter->relative_path); ++ + if (S_ISDIR(iter->st.st_mode)) { + mkdir_if_missing(dest->buf, 0777); + continue; +diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh +index 2f7be23044..9d32f1c4a4 100755 +--- a/t/t5604-clone-reference.sh ++++ b/t/t5604-clone-reference.sh +@@ -300,8 +300,6 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file + ln -s ../an-object $obj && + + cd ../ && +- find . -type f | sort >../../../T.objects-files.raw && +- find . -type l | sort >../../../T.objects-symlinks.raw && + echo unknown_content >unknown_file + ) && + git -C T fsck && +@@ -310,19 +308,27 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file + + + test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at objects/' ' +- for option in --local --no-hardlinks --shared --dissociate ++ # None of these options work when cloning locally, since T has ++ # symlinks in its `$GIT_DIR/objects` directory ++ for option in --local --no-hardlinks --dissociate + do +- git clone $option T T$option || return 1 && +- git -C T$option fsck || return 1 && +- git -C T$option rev-list --all --objects >T$option.objects && +- test_cmp T.objects T$option.objects && +- ( +- cd T$option/.git/objects && +- find . -type f | sort >../../../T$option.objects-files.raw && +- find . -type l | sort >../../../T$option.objects-symlinks.raw +- ) ++ test_must_fail git clone $option T T$option 2>err || return 1 && ++ test_i18ngrep "symlink.*exists" err || return 1 + done && + ++ # But `--shared` clones should still work, even when specifying ++ # a local path *and* that repository has symlinks present in its ++ # `$GIT_DIR/objects` directory. ++ git clone --shared T T--shared && ++ git -C T--shared fsck && ++ git -C T--shared rev-list --all --objects >T--shared.objects && ++ test_cmp T.objects T--shared.objects && ++ ( ++ cd T--shared/.git/objects && ++ find . -type f | sort >../../../T--shared.objects-files.raw && ++ find . -type l | sort >../../../T--shared.objects-symlinks.raw ++ ) && ++ + for raw in $(ls T*.raw) + do + sed -e "s!/../!/Y/!; s![0-9a-f]\{38,\}!Z!" -e "/commit-graph/d" \ +@@ -330,26 +336,6 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje + sort $raw.de-sha-1 >$raw.de-sha || return 1 + done && + +- cat >expected-files <<-EOF && +- ./Y/Z +- ./Y/Z +- ./Y/Z +- ./a-loose-dir/Z +- ./an-object +- ./info/packs +- ./pack/pack-Z.idx +- ./pack/pack-Z.pack +- ./packs/pack-Z.idx +- ./packs/pack-Z.pack +- ./unknown_file +- EOF +- +- for option in --local --no-hardlinks --dissociate +- do +- test_cmp expected-files T$option.objects-files.raw.de-sha || return 1 && +- test_must_be_empty T$option.objects-symlinks.raw.de-sha || return 1 +- done && +- + echo ./info/alternates >expected-files && + test_cmp expected-files T--shared.objects-files.raw && + test_must_be_empty T--shared.objects-symlinks.raw +-- +2.27.0 + diff --git a/backport-CVE-2022-39260-alias.c-reject-too-long-cmdline-strings-in-split_cmd.patch b/backport-CVE-2022-39260-alias.c-reject-too-long-cmdline-strings-in-split_cmd.patch new file mode 100644 index 0000000000000000000000000000000000000000..2c12b6e1acd6d5f122e88b1cb4aa43bf7c51dbcb --- /dev/null +++ b/backport-CVE-2022-39260-alias.c-reject-too-long-cmdline-strings-in-split_cmd.patch @@ -0,0 +1,85 @@ +From 0ca6ead81edd4fb1984b69aae87c1189e3025530 Mon Sep 17 00:00:00 2001 +From: Kevin Backhouse +Date: Wed, 28 Sep 2022 18:53:32 -0400 +Subject: [PATCH] alias.c: reject too-long cmdline strings in split_cmdline() + +This function improperly uses an int to represent the number of entries +in the resulting argument array. This allows a malicious actor to +intentionally overflow the return value, leading to arbitrary heap +writes. + +Because the resulting argv array is typically passed to execv(), it may +be possible to leverage this attack to gain remote code execution on a +victim machine. This was almost certainly the case for certain +configurations of git-shell until the previous commit limited the size +of input it would accept. Other calls to split_cmdline() are typically +limited by the size of argv the OS is willing to hand us, so are +similarly protected. + +So this is not strictly fixing a known vulnerability, but is a hardening +of the function that is worth doing to protect against possible unknown +vulnerabilities. + +One approach to fixing this would be modifying the signature of +`split_cmdline()` to look something like: + + int split_cmdline(char *cmdline, const char ***argv, size_t *argc); + +Where the return value of `split_cmdline()` is negative for errors, and +zero otherwise. If non-NULL, the `*argc` pointer is modified to contain +the size of the `**argv` array. + +But this implies an absurdly large `argv` array, which more than likely +larger than the system's argument limit. So even if split_cmdline() +allowed this, it would fail immediately afterwards when we called +execv(). So instead of converting all of `split_cmdline()`'s callers to +work with `size_t` types in this patch, instead pursue the minimal fix +here to prevent ever returning an array with more than INT_MAX entries +in it. + +Signed-off-by: Kevin Backhouse +Signed-off-by: Taylor Blau +Signed-off-by: Jeff King +Signed-off-by: Taylor Blau +--- + alias.c | 11 +++++++++-- + 1 file changed, 9 insertions(+), 2 deletions(-) + +diff --git a/alias.c b/alias.c +index c471538020..00abde0817 100644 +--- a/alias.c ++++ b/alias.c +@@ -46,14 +46,16 @@ void list_aliases(struct string_list *list) + + #define SPLIT_CMDLINE_BAD_ENDING 1 + #define SPLIT_CMDLINE_UNCLOSED_QUOTE 2 ++#define SPLIT_CMDLINE_ARGC_OVERFLOW 3 + static const char *split_cmdline_errors[] = { + N_("cmdline ends with \\"), +- N_("unclosed quote") ++ N_("unclosed quote"), ++ N_("too many arguments"), + }; + + int split_cmdline(char *cmdline, const char ***argv) + { +- int src, dst, count = 0, size = 16; ++ size_t src, dst, count = 0, size = 16; + char quoted = 0; + + ALLOC_ARRAY(*argv, size); +@@ -96,6 +98,11 @@ int split_cmdline(char *cmdline, const char ***argv) + return -SPLIT_CMDLINE_UNCLOSED_QUOTE; + } + ++ if (count >= INT_MAX) { ++ FREE_AND_NULL(*argv); ++ return -SPLIT_CMDLINE_ARGC_OVERFLOW; ++ } ++ + ALLOC_GROW(*argv, count + 1, size); + (*argv)[count] = NULL; + +-- +2.27.0 + diff --git a/backport-CVE-2022-39260-shell-add-basic-tests.patch b/backport-CVE-2022-39260-shell-add-basic-tests.patch new file mode 100644 index 0000000000000000000000000000000000000000..49c75067cf4574c5c8819f599bff864c90126c6b --- /dev/null +++ b/backport-CVE-2022-39260-shell-add-basic-tests.patch @@ -0,0 +1,57 @@ +From 32696a4cbe90929ae79ea442f5102c513ce3dfaa Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 28 Sep 2022 18:50:36 -0400 +Subject: [PATCH] shell: add basic tests + +We have no tests of even basic functionality of git-shell. Let's add a +couple of obvious ones. This will serve as a framework for adding tests +for new things we fix, as well as making sure we don't screw anything up +too badly while doing so. + +Signed-off-by: Jeff King +Signed-off-by: Taylor Blau +--- + t/t9850-shell.sh | 31 +++++++++++++++++++++++++++++++ + 1 file changed, 31 insertions(+) + create mode 100755 t/t9850-shell.sh + +diff --git a/t/t9850-shell.sh b/t/t9850-shell.sh +new file mode 100755 +index 0000000000..2af476c3af +--- /dev/null ++++ b/t/t9850-shell.sh +@@ -0,0 +1,31 @@ ++#!/bin/sh ++ ++test_description='git shell tests' ++. ./test-lib.sh ++ ++test_expect_success 'shell allows upload-pack' ' ++ printf 0000 >input && ++ git upload-pack . expect && ++ git shell -c "git-upload-pack $SQ.$SQ" actual && ++ test_cmp expect actual ++' ++ ++test_expect_success 'shell forbids other commands' ' ++ test_must_fail git shell -c "git config foo.bar baz" ++' ++ ++test_expect_success 'shell forbids interactive use by default' ' ++ test_must_fail git shell ++' ++ ++test_expect_success 'shell allows interactive command' ' ++ mkdir git-shell-commands && ++ write_script git-shell-commands/ping <<-\EOF && ++ echo pong ++ EOF ++ echo pong >expect && ++ echo ping | git shell >actual && ++ test_cmp expect actual ++' ++ ++test_done +-- +2.27.0 + diff --git a/backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch b/backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch new file mode 100644 index 0000000000000000000000000000000000000000..40f99ad32e3c7680f8b8a5f1416fc697c53eff00 --- /dev/null +++ b/backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch @@ -0,0 +1,153 @@ +From 71ad7fe1bcec2a115bd0ab187240348358aa7f21 Mon Sep 17 00:00:00 2001 +From: Jeff King +Date: Wed, 28 Sep 2022 18:52:48 -0400 +Subject: [PATCH] shell: limit size of interactive commands + +When git-shell is run in interactive mode (which must be enabled by +creating $HOME/git-shell-commands), it reads commands from stdin, one +per line, and executes them. + +We read the commands with git_read_line_interactively(), which uses a +strbuf under the hood. That means we'll accept an input of arbitrary +size (limited only by how much heap we can allocate). That creates two +problems: + + - the rest of the code is not prepared to handle large inputs. The + most serious issue here is that split_cmdline() uses "int" for most + of its types, which can lead to integer overflow and out-of-bounds + array reads and writes. But even with that fixed, we assume that we + can feed the command name to snprintf() (via xstrfmt()), which is + stuck for historical reasons using "int", and causes it to fail (and + even trigger a BUG() call). + + - since the point of git-shell is to take input from untrusted or + semi-trusted clients, it's a mild denial-of-service. We'll allocate + as many bytes as the client sends us (actually twice as many, since + we immediately duplicate the buffer). + +We can fix both by just limiting the amount of per-command input we're +willing to receive. + +We should also fix split_cmdline(), of course, which is an accident +waiting to happen, but that can come on top. Most calls to +split_cmdline(), including the other one in git-shell, are OK because +they are reading from an OS-provided argv, which is limited in practice. +This patch should eliminate the immediate vulnerabilities. + +I picked 4MB as an arbitrary limit. It's big enough that nobody should +ever run into it in practice (since the point is to run the commands via +exec, we're subject to OS limits which are typically much lower). But +it's small enough that allocating it isn't that big a deal. + +The code is mostly just swapping out fgets() for the strbuf call, but we +have to add a few niceties like flushing and trimming line endings. We +could simplify things further by putting the buffer on the stack, but +4MB is probably a bit much there. Note that we'll _always_ allocate 4MB, +which for normal, non-malicious requests is more than we would before +this patch. But on the other hand, other git programs are happy to use +96MB for a delta cache. And since we'd never touch most of those pages, +on a lazy-allocating OS like Linux they won't even get allocated to +actual RAM. + +The ideal would be a version of strbuf_getline() that accepted a maximum +value. But for a minimal vulnerability fix, let's keep things localized +and simple. We can always refactor further on top. + +The included test fails in an obvious way with ASan or UBSan (which +notice the integer overflow and out-of-bounds reads). Without them, it +fails in a less obvious way: we may segfault, or we may try to xstrfmt() +a long string, leading to a BUG(). Either way, it fails reliably before +this patch, and passes with it. Note that we don't need an EXPENSIVE +prereq on it. It does take 10-15s to fail before this patch, but with +the new limit, we fail almost immediately (and the perl process +generating 2GB of data exits via SIGPIPE). + +Signed-off-by: Jeff King +Signed-off-by: Taylor Blau +--- + shell.c | 34 ++++++++++++++++++++++++++++++---- + t/t9850-shell.sh | 6 ++++++ + 2 files changed, 36 insertions(+), 4 deletions(-) + +diff --git a/shell.c b/shell.c +index cef7ffdc9e..02cfd9627f 100644 +--- a/shell.c ++++ b/shell.c +@@ -47,6 +47,8 @@ static void cd_to_homedir(void) + die("could not chdir to user's home directory"); + } + ++#define MAX_INTERACTIVE_COMMAND (4*1024*1024) ++ + static void run_shell(void) + { + int done = 0; +@@ -67,22 +69,46 @@ static void run_shell(void) + run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE); + + do { +- struct strbuf line = STRBUF_INIT; + const char *prog; + char *full_cmd; + char *rawargs; ++ size_t len; + char *split_args; + const char **argv; + int code; + int count; + + fprintf(stderr, "git> "); +- if (git_read_line_interactively(&line) == EOF) { ++ ++ /* ++ * Avoid using a strbuf or git_read_line_interactively() here. ++ * We don't want to allocate arbitrary amounts of memory on ++ * behalf of a possibly untrusted client, and we're subject to ++ * OS limits on command length anyway. ++ */ ++ fflush(stdout); ++ rawargs = xmalloc(MAX_INTERACTIVE_COMMAND); ++ if (!fgets(rawargs, MAX_INTERACTIVE_COMMAND, stdin)) { + fprintf(stderr, "\n"); +- strbuf_release(&line); ++ free(rawargs); + break; + } +- rawargs = strbuf_detach(&line, NULL); ++ len = strlen(rawargs); ++ ++ /* ++ * If we truncated due to our input buffer size, reject the ++ * command. That's better than running bogus input, and ++ * there's a good chance it's just malicious garbage anyway. ++ */ ++ if (len >= MAX_INTERACTIVE_COMMAND - 1) ++ die("invalid command format: input too long"); ++ ++ if (len > 0 && rawargs[len - 1] == '\n') { ++ if (--len > 0 && rawargs[len - 1] == '\r') ++ --len; ++ rawargs[len] = '\0'; ++ } ++ + split_args = xstrdup(rawargs); + count = split_cmdline(split_args, &argv); + if (count < 0) { +diff --git a/t/t9850-shell.sh b/t/t9850-shell.sh +index 2af476c3af..cfc71c3bd4 100755 +--- a/t/t9850-shell.sh ++++ b/t/t9850-shell.sh +@@ -28,4 +28,10 @@ test_expect_success 'shell allows interactive command' ' + test_cmp expect actual + ' + ++test_expect_success 'shell complains of overlong commands' ' ++ perl -e "print \"a\" x 2**12 for (0..2**19)" | ++ test_must_fail git shell 2>err && ++ grep "too long" err ++' ++ + test_done +-- +2.27.0 + diff --git a/git.spec b/git.spec index 89dedb74eb22c744cf77b2d50e020aa6ec24ef90..5b0d69913aca3ca35e5e44241454d6d451771534 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.27.0 -Release: 8 +Release: 9 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -24,6 +24,10 @@ Patch9: backport-t0033-add-tests-for-safe.directory.patch Patch10: backport-0005-CVE-2022-24765.patch Patch11: backport-0006-CVE-2022-24765.patch Patch12: backport-CVE-2022-29187.patch +Patch13: backport-CVE-2022-39253-builtin-clone.c-disallow-local-clones-with-symlinks.patch +Patch14: backport-CVE-2022-39260-shell-add-basic-tests.patch +Patch15: backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch +Patch16: backport-CVE-2022-39260-alias.c-reject-too-long-cmdline-strings-in-split_cmd.patch BuildRequires: gcc gettext BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils @@ -273,6 +277,12 @@ make %{?_smp_mflags} test %{_mandir}/man7/git*.7.* %changelog +* Fri Oct 21 2022 fuanan - 2.27.0-9 +- Type:CVE +- ID:CVE-2022-39253 CVE-2022-39260 +- SUG:NA +- DESC:Fix CVE-2022-39253 CVE-2022-39260 + * Mon Jul 18 2022 fuanan - 2.27.0-8 - Type:CVE - ID:CVE-2022-29187