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 a6ae7dd4a73e961fb50891aeca67d2e679b30176..224a767575dd8fe0d869ee6195da2ef97692aab7 100644 --- a/git.spec +++ b/git.spec @@ -1,7 +1,7 @@ %global gitexecdir %{_libexecdir}/git-core Name: git Version: 2.27.0 -Release: 10 +Release: 11 Summary: A popular and widely used Version Control System License: GPLv2+ or LGPLv2.1 URL: https://git-scm.com/ @@ -24,6 +24,9 @@ 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-39260-shell-add-basic-tests.patch +Patch14: backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch +Patch15: 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 +276,12 @@ make %{?_smp_mflags} test %{_mandir}/man7/git*.7.* %changelog +* Fri Oct 21 2022 fuanan - 2.27.0-11 +- Type:CVE +- ID:CVE-2022-39260 +- SUG:NA +- DESC:Fix CVE-2022-39260 + * Mon Sep 19 2022 fuanan - 2.27.0-10 - revert "add subpackage git-core"