packages/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!81 Fix CVE-2022-39260

Browse Source 
From: @fly_fzc 
Reviewed-by: @overweight 
Signed-off-by: @overweight
This commit is contained in:
openeuler-ci-bot 2022-10-24 01:20:52 +00:00 committed by Gitee
commit cbb5bfd1dd
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
4 changed files with 305 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
backport-CVE-2022-39260-alias.c-reject-too-long-cmdline-strings-in-split_cmd.patch Normal file
View File

@ -0,0 +1,85 @@
From 0ca6ead81edd4fb1984b69aae87c1189e3025530 Mon Sep 17 00:00:00 2001
From: Kevin Backhouse <kevinbackhouse@github.com>
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 <kevinbackhouse@github.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
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

57
backport-CVE-2022-39260-shell-add-basic-tests.patch Normal file
View File

@ -0,0 +1,57 @@
From 32696a4cbe90929ae79ea442f5102c513ce3dfaa Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
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 . <input >expect &&
+ git shell -c "git-upload-pack $SQ.$SQ" <input >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

153
backport-CVE-2022-39260-shell-limit-size-of-interactive-commands.patch Normal file
View File

@ -0,0 +1,153 @@
From 71ad7fe1bcec2a115bd0ab187240348358aa7f21 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
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 <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
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

11
git.spec
View File

@ -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 <fuanan3@h-partners.com> - 2.27.0-11
- Type:CVE
- ID:CVE-2022-39260
- SUG:NA
- DESC:Fix CVE-2022-39260
* Mon Sep 19 2022 fuanan <fuanan3@h-partners.com> - 2.27.0-10
- revert "add subpackage git-core"