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

Fix CVE-2023-25652 CVE-2023-29007 CVE-2023-25815

Browse Source
This commit is contained in:
fly_fzc 2023-04-26 11:01:40 +08:00
parent da474d370b
commit 55d44eafa6
4 changed files with 545 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
backport-CVE-2023-25652-apply-reject-overwrite-existing-.rej-symlink-if-it-e.patch Normal file
View File

@ -0,0 +1,90 @@
From 9db05711c98efc14f414d4c87135a34c13586e0b Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 9 Mar 2023 16:02:54 +0100
Subject: [PATCH] apply --reject: overwrite existing `.rej` symlink if it
exists
The `git apply --reject` is expected to write out `.rej` files in case
one or more hunks fail to apply cleanly. Historically, the command
overwrites any existing `.rej` files. The idea being that
apply/reject/edit cycles are relatively common, and the generated `.rej`
files are not considered precious.
But the command does not overwrite existing `.rej` symbolic links, and
instead follows them. This is unsafe because the same patch could
potentially create such a symbolic link and point at arbitrary paths
outside the current worktree, and `git apply` would write the contents
of the `.rej` file into that location.
Therefore, let's make sure that any existing `.rej` file or symbolic
link is removed before writing it.
Reported-by: RyotaK <ryotak.mail@gmail.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Linus Torvalds <torvalds@linuxfoundation.org>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
apply.c | 14 ++++++++++++--
t/t4115-apply-symlink.sh | 15 +++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index d80382c940..6634e9c510 100644
--- a/apply.c
+++ b/apply.c
@@ -4558,7 +4558,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
FILE *rej;
char namebuf[PATH_MAX];
struct fragment *frag;
- int cnt = 0;
+ int fd, cnt = 0;
struct strbuf sb = STRBUF_INIT;
for (cnt = 0, frag = patch->fragments; frag; frag = frag->next) {
@@ -4598,7 +4598,17 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)
memcpy(namebuf, patch->new_name, cnt);
memcpy(namebuf + cnt, ".rej", 5);
- rej = fopen(namebuf, "w");
+ fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+ if (fd < 0) {
+ if (errno != EEXIST)
+ return error_errno(_("cannot open %s"), namebuf);
+ if (unlink(namebuf))
+ return error_errno(_("cannot unlink '%s'"), namebuf);
+ fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
+ if (fd < 0)
+ return error_errno(_("cannot open %s"), namebuf);
+ }
+ rej = fdopen(fd, "w");
if (!rej)
return error_errno(_("cannot open %s"), namebuf);
diff --git a/t/t4115-apply-symlink.sh b/t/t4115-apply-symlink.sh
index 14e0f4d705..2d03c4e4d1 100755
--- a/t/t4115-apply-symlink.sh
+++ b/t/t4115-apply-symlink.sh
@@ -125,4 +125,19 @@ test_expect_success SYMLINKS 'symlink escape when deleting file' '
test_path_is_file .git/delete-me
'
+test_expect_success SYMLINKS '--reject removes .rej symlink if it exists' '
+ test_when_finished "git reset --hard && git clean -dfx" &&
+
+ test_commit file &&
+ echo modified >file.t &&
+ git diff -- file.t >patch &&
+ echo modified-again >file.t &&
+
+ ln -s foo file.t.rej &&
+ test_must_fail git apply patch --reject 2>err &&
+ test_i18ngrep "Rejected hunk" err &&
+ test_path_is_missing foo &&
+ test_path_is_file file.t.rej
+'
+
test_done
--
2.27.0

89
backport-CVE-2023-25815-gettext-avoid-using-gettext-if-the-locale-dir-is-not.patch Normal file
View File

@ -0,0 +1,89 @@
From c4137be0f5a6edf9a9044e6e43ecf4468c7a4046 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 22 Feb 2023 12:40:55 +0100
Subject: [PATCH] gettext: avoid using gettext if the locale dir is not present
In cc5e1bf99247 (gettext: avoid initialization if the locale dir is not
present, 2018-04-21) Git was taught to avoid a costly gettext start-up
when there are not even any localized messages to work with.
But we still called `gettext()` and `ngettext()` functions.
Which caused a problem in Git for Windows when the libgettext that is
consumed from the MSYS2 project stopped using a runtime prefix in
https://github.com/msys2/MINGW-packages/pull/10461
Due to that change, we now use an unintialized gettext machinery that
might get auto-initialized _using an unintended locale directory_:
`C:\mingw64\share\locale`.
Let's record the fact when the gettext initialization was skipped, and
skip calling the gettext functions accordingly.
This addresses CVE-2023-25815.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
gettext.c | 4 ++++
gettext.h | 7 ++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/gettext.c b/gettext.c
index 1b564216d0..610d402fe7 100644
--- a/gettext.c
+++ b/gettext.c
@@ -109,6 +109,8 @@ static void init_gettext_charset(const char *domain)
setlocale(LC_CTYPE, "C");
}
+int git_gettext_enabled = 0;
+
void git_setup_gettext(void)
{
const char *podir = getenv(GIT_TEXT_DOMAIN_DIR_ENVIRONMENT);
@@ -130,6 +132,8 @@ void git_setup_gettext(void)
init_gettext_charset("git");
textdomain("git");
+ git_gettext_enabled = 1;
+
free(p);
}
diff --git a/gettext.h b/gettext.h
index bee52eb113..b96ab9d340 100644
--- a/gettext.h
+++ b/gettext.h
@@ -31,9 +31,11 @@
#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
#ifndef NO_GETTEXT
+extern int git_gettext_enabled;
void git_setup_gettext(void);
int gettext_width(const char *s);
#else
+#define git_gettext_enabled (0)
static inline void git_setup_gettext(void)
{
}
@@ -48,6 +50,8 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
{
if (!*msgid)
return "";
+ if (!git_gettext_enabled)
+ return msgid;
return gettext(msgid);
}
@@ -56,6 +59,8 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
const char *Q_(const char *msgid, const char *plu, unsigned long n)
{
+ if (!git_gettext_enabled)
+ return n == 1 ? msgid : plu;
return ngettext(msgid, plu, n);
}
--
2.27.0

356
backport-CVE-2023-29007.patch Normal file
View File

@ -0,0 +1,356 @@
From 29198213c9163c1d552ee2bdbf78d2b09ccc98b8 Mon Sep 17 00:00:00 2001
From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 6 Apr 2023 11:42:03 -0400
Subject: [PATCH 1/4] t1300: demonstrate failure when renaming sections with
long lines
When renaming a configuration section which has an entry whose length
exceeds the size of our buffer in config.c's implementation of
`git_config_copy_or_rename_section_in_file()`, Git will incorrectly
form a new configuration section with part of the data in the section
being removed.
In this instance, our first configuration file looks something like:
[b]
c = d <spaces> [a] e = f
[a]
g = h
Here, we have two configuration values, "b.c", and "a.g". The value "[a]
e = f" belongs to the configuration value "b.c", and does not form its
own section.
However, when renaming the section 'a' to 'xyz', Git will write back
"[xyz]\ne = f", but "[xyz]" is still attached to the value of "b.c",
which is why "e = f" on its own line becomes a new entry called "b.e".
A slightly different example embeds the section being renamed within
another section.
Demonstrate this failure in a test in t1300, which we will fix in the
following commit.
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/t1300-config.sh | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 1a4156c70434f3..cd8f744160e8ba 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -613,6 +613,26 @@ test_expect_success 'renaming to bogus section is rejected' '
test_must_fail git config --rename-section branch.zwei "bogus name"
'
+test_expect_failure 'renaming a section with a long line' '
+ {
+ printf "[b]\\n" &&
+ printf " c = d %1024s [a] e = f\\n" " " &&
+ printf "[a] g = h\\n"
+ } >y &&
+ git config -f y --rename-section a xyz &&
+ test_must_fail git config -f y b.e
+'
+
+test_expect_failure 'renaming an embedded section with a long line' '
+ {
+ printf "[b]\\n" &&
+ printf " c = d %1024s [a] [foo] e = f\\n" " " &&
+ printf "[a] g = h\\n"
+ } >y &&
+ git config -f y --rename-section a xyz &&
+ test_must_fail git config -f y foo.e
+'
+
cat >> .git/config << EOF
[branch "zwei"] a = 1 [branch "vier"]
EOF
From a5bb10fd5e74101e7c07da93e7c32bbe60f6173a Mon Sep 17 00:00:00 2001
From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 6 Apr 2023 14:07:58 -0400
Subject: [PATCH 2/4] config: avoid fixed-sized buffer when renaming/deleting a
section
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When renaming (or deleting) a section of configuration, Git uses the
function `git_config_copy_or_rename_section_in_file()` to rewrite the
configuration file after applying the rename or deletion to the given
section.
To do this, Git repeatedly calls `fgets()` to read the existing
configuration data into a fixed size buffer.
When the configuration value under `old_name` exceeds the size of the
buffer, we will call `fgets()` an additional time even if there is no
newline in the configuration file, since our read length is capped at
`sizeof(buf)`.
If the first character of the buffer (after zero or more characters
satisfying `isspace()`) is a '[', Git will incorrectly treat it as
beginning a new section when the original section is being removed. In
other words, a configuration value satisfying this criteria can
incorrectly be considered as a new secftion instead of a variable in the
original section.
Avoid this issue by using a variable-width buffer in the form of a
strbuf rather than a fixed-with region on the stack. A couple of small
points worth noting:
- Using a strbuf will cause us to allocate arbitrary sizes to match
the length of each line. In practice, we don't expect any
reasonable configuration files to have lines that long, and a
bandaid will be introduced in a later patch to ensure that this is
the case.
- We are using strbuf_getwholeline() here instead of strbuf_getline()
in order to match `fgets()`'s behavior of leaving the trailing LF
character on the buffer (as well as a trailing NUL).
This could be changed later, but using strbuf_getwholeline() changes
the least about this function's implementation, so it is picked as
the safest path.
- It is temping to want to replace the loop to skip over characters
matching isspace() at the beginning of the buffer with a convenience
function like `strbuf_ltrim()`. But this is the wrong approach for a
couple of reasons:
First, it involves a potentially large and expensive `memmove()`
which we would like to avoid. Second, and more importantly, we also
*do* want to preserve those spaces to avoid changing the output of
other sections.
In all, this patch is a minimal replacement of the fixed-width buffer in
`git_config_copy_or_rename_section_in_file()` to instead use a `struct
strbuf`.
Reported-by: André Baptista <andre@ethiack.com>
Reported-by: Vítor Pinho <vitor@ethiack.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
config.c | 13 +++++++------
t/t1300-config.sh | 4 ++--
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/config.c b/config.c
index 1137bd73aff07c..524347676d0da0 100644
--- a/config.c
+++ b/config.c
@@ -3091,7 +3091,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
char *filename_buf = NULL;
struct lock_file lock = LOCK_INIT;
int out_fd;
- char buf[1024];
+ struct strbuf buf = STRBUF_INIT;
FILE *config_file = NULL;
struct stat st;
struct strbuf copystr = STRBUF_INIT;
@@ -3132,14 +3132,14 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
goto out;
}
- while (fgets(buf, sizeof(buf), config_file)) {
+ while (!strbuf_getwholeline(&buf, config_file, '\n')) {
unsigned i;
int length;
int is_section = 0;
- char *output = buf;
- for (i = 0; buf[i] && isspace(buf[i]); i++)
+ char *output = buf.buf;
+ for (i = 0; buf.buf[i] && isspace(buf.buf[i]); i++)
; /* do nothing */
- if (buf[i] == '[') {
+ if (buf.buf[i] == '[') {
/* it's a section */
int offset;
is_section = 1;
@@ -3158,7 +3158,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
strbuf_reset(&copystr);
}
- offset = section_name_match(&buf[i], old_name);
+ offset = section_name_match(&buf.buf[i], old_name);
if (offset > 0) {
ret++;
if (!new_name) {
@@ -3233,6 +3233,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
out_no_rollback:
free(filename_buf);
config_store_data_clear(&store);
+ strbuf_release(&buf);
return ret;
}
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index cd8f744160e8ba..24c13b91dbd669 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -613,7 +613,7 @@ test_expect_success 'renaming to bogus section is rejected' '
test_must_fail git config --rename-section branch.zwei "bogus name"
'
-test_expect_failure 'renaming a section with a long line' '
+test_expect_success 'renaming a section with a long line' '
{
printf "[b]\\n" &&
printf " c = d %1024s [a] e = f\\n" " " &&
@@ -623,7 +623,7 @@ test_expect_failure 'renaming a section with a long line' '
test_must_fail git config -f y b.e
'
-test_expect_failure 'renaming an embedded section with a long line' '
+test_expect_success 'renaming an embedded section with a long line' '
{
printf "[b]\\n" &&
printf " c = d %1024s [a] [foo] e = f\\n" " " &&
From e91cfe6085c4a61372d1f800b473b73b8d225d0d Mon Sep 17 00:00:00 2001
From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 6 Apr 2023 14:28:53 -0400
Subject: [PATCH 3/4] config.c: avoid integer truncation in
`copy_or_rename_section_in_file()`
There are a couple of spots within `copy_or_rename_section_in_file()`
that incorrectly use an `int` to track an offset within a string, which
may truncate or wrap around to a negative value.
Historically it was impossible to have a line longer than 1024 bytes
anyway, since we used fgets() with a fixed-size buffer of exactly that
length. But the recent change to use a strbuf permits us to read lines
of arbitrary length, so it's possible for a malicious input to cause us
to overflow past INT_MAX and do an out-of-bounds array read.
Practically speaking, however, this should never happen, since it
requires 2GB section names or values, which are unrealistic in
non-malicious circumstances.
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
config.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c
index 524347676d0da0..e4189aa2d79f62 100644
--- a/config.c
+++ b/config.c
@@ -3027,9 +3027,10 @@ void git_config_set_multivar(const char *key, const char *value,
flags);
}
-static int section_name_match (const char *buf, const char *name)
+static size_t section_name_match (const char *buf, const char *name)
{
- int i = 0, j = 0, dot = 0;
+ size_t i = 0, j = 0;
+ int dot = 0;
if (buf[i] != '[')
return 0;
for (i = 1; buf[i] && buf[i] != ']'; i++) {
@@ -3133,15 +3134,14 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
}
while (!strbuf_getwholeline(&buf, config_file, '\n')) {
- unsigned i;
- int length;
+ size_t i, length;
int is_section = 0;
char *output = buf.buf;
for (i = 0; buf.buf[i] && isspace(buf.buf[i]); i++)
; /* do nothing */
if (buf.buf[i] == '[') {
/* it's a section */
- int offset;
+ size_t offset;
is_section = 1;
/*
From 3bb3d6bac5f2b496dfa2862dc1a84cbfa9b4449a Mon Sep 17 00:00:00 2001
From: Taylor Blau <me@ttaylorr.com>
Date: Wed, 12 Apr 2023 19:18:28 -0400
Subject: [PATCH 4/4] config.c: disallow overly-long lines in
`copy_or_rename_section_in_file()`
As a defense-in-depth measure to guard against any potentially-unknown
buffer overflows in `copy_or_rename_section_in_file()`, refuse to work
with overly-long lines in a gitconfig.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
config.c | 13 +++++++++++++
t/t1300-config.sh | 10 ++++++++++
2 files changed, 23 insertions(+)
diff --git a/config.c b/config.c
index e4189aa2d79f62..b8194dfd8a78af 100644
--- a/config.c
+++ b/config.c
@@ -3083,6 +3083,8 @@ static int section_name_is_ok(const char *name)
return 1;
}
+#define GIT_CONFIG_MAX_LINE_LEN (512 * 1024)
+
/* if new_name == NULL, the section is removed instead */
static int git_config_copy_or_rename_section_in_file(const char *config_filename,
const char *old_name,
@@ -3097,6 +3099,7 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
struct stat st;
struct strbuf copystr = STRBUF_INIT;
struct config_store_data store;
+ uint32_t line_nr = 0;
memset(&store, 0, sizeof(store));
@@ -3137,6 +3140,16 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
size_t i, length;
int is_section = 0;
char *output = buf.buf;
+
+ line_nr++;
+
+ if (buf.len >= GIT_CONFIG_MAX_LINE_LEN) {
+ ret = error(_("refusing to work with overly long line "
+ "in '%s' on line %"PRIuMAX),
+ config_filename, (uintmax_t)line_nr);
+ goto out;
+ }
+
for (i = 0; buf.buf[i] && isspace(buf.buf[i]); i++)
; /* do nothing */
if (buf.buf[i] == '[') {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 24c13b91dbd669..de564cb8e587a6 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -633,6 +633,16 @@ test_expect_success 'renaming an embedded section with a long line' '
test_must_fail git config -f y foo.e
'
+test_expect_success 'renaming a section with an overly-long line' '
+ {
+ printf "[b]\\n" &&
+ printf " c = d %525000s e" " " &&
+ printf "[a] g = h\\n"
+ } >y &&
+ test_must_fail git config -f y --rename-section a xyz 2>err &&
+ test_i18ngrep "refusing to work with overly long line in .y. on line 2" err
+'
+
cat >> .git/config << EOF
[branch "zwei"] a = 1 [branch "vier"]
EOF

11
git.spec
View File

@ -1,7 +1,7 @@
%global gitexecdir %{_libexecdir}/git-core %global gitexecdir %{_libexecdir}/git-core
Name: git Name: git
Version: 2.39.1 Version: 2.39.1
Release: 4 Release: 5
Summary: A popular and widely used Version Control System Summary: A popular and widely used Version Control System
License: GPLv2+ or LGPLv2.1 License: GPLv2+ or LGPLv2.1
URL: https://git-scm.com/ URL: https://git-scm.com/
@ -22,6 +22,9 @@ Patch6: backport-CVE-2023-22490-t5619-demonstrate-clone_local-with-ambiguous
Patch7: backport-CVE-2023-22490-clone-delay-picking-a-transport-until-after-get_repo.patch Patch7: backport-CVE-2023-22490-clone-delay-picking-a-transport-until-after-get_repo.patch
Patch8: backport-CVE-2023-22490-dir-iterator-prevent-top-level-symlinks-without-FOLL.patch Patch8: backport-CVE-2023-22490-dir-iterator-prevent-top-level-symlinks-without-FOLL.patch
Patch9: backport-CVE-2023-23946-apply-fix-writing-behind-newly-created-symbolic-link.patch Patch9: backport-CVE-2023-23946-apply-fix-writing-behind-newly-created-symbolic-link.patch
Patch10: backport-CVE-2023-25652-apply-reject-overwrite-existing-.rej-symlink-if-it-e.patch
Patch11: backport-CVE-2023-29007.patch
Patch12: backport-CVE-2023-25815-gettext-avoid-using-gettext-if-the-locale-dir-is-not.patch
BuildRequires: gcc gettext BuildRequires: gcc gettext
BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils BuildRequires: openssl-devel libcurl-devel expat-devel systemd asciidoc xmlto glib2-devel libsecret-devel pcre-devel desktop-file-utils
@ -300,6 +303,12 @@ make %{?_smp_mflags} test
%{_mandir}/man7/git*.7.* %{_mandir}/man7/git*.7.*
%changelog %changelog
* Wed Apr 26 2023 fuanan <fuanan3@h-partners.com> - 2.39.1-5
- Type:CVE
- ID:CVE-2023-25652 CVE-2023-29007 CVE-2023-25815
- SUG:NA
- DESC:Fix CVE-2023-25652 CVE-2023-29007 CVE-2023-25815
* Wed Mar 22 2023 lijian <lijian2@kylinos.cn> - 2.39.1-4 * Wed Mar 22 2023 lijian <lijian2@kylinos.cn> - 2.39.1-4
- Delete git-core dir in %files git-bin-files - Delete git-core dir in %files git-bin-files