diff mbox series

[dunfell] git: fix CVE-2023-25652

Message ID 20230515052517.29549-1-hprajapati@mvista.com
State New, archived
Headers show
Series [dunfell] git: fix CVE-2023-25652 | expand

Commit Message

Hitendra Prajapati May 15, 2023, 5:25 a.m. UTC
Git is a revision control system. Prior to versions 2.30.9, 2.31.8, 2.32.7,
2.33.8, 2.34.8, 2.35.8, 2.36.6, 2.37.7, 2.38.5, 2.39.3, and 2.40.1, by feeding
specially crafted input to `git apply --reject`, a path outside the working
tree can be overwritten with partially controlled contents (corresponding to
the rejected hunk(s) from the given patch). A fix is available in versions
2.30.9, 2.31.8, 2.32.7, 2.33.8, 2.34.8, 2.35.8, 2.36.6, 2.37.7, 2.38.5, 2.39.3,
and 2.40.1. As a workaround, avoid using `git apply` with `--reject` when applying
patches from an untrusted source. Use `git apply --stat` to inspect a patch before
applying; avoid applying one that create a conflict where a link corresponding to
the `*.rej` file exists.

References:
https://nvd.nist.gov/vuln/detail/CVE-2023-25652

Upstream-Status: Backport from https://github.com/git/git/commit/9db05711c98efc14f414d4c87135a34c13586e0b

Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
---
 .../git/files/CVE-2023-25652.patch            | 95 +++++++++++++++++++
 meta/recipes-devtools/git/git.inc             |  1 +
 2 files changed, 96 insertions(+)
 create mode 100644 meta/recipes-devtools/git/files/CVE-2023-25652.patch

Comments

Steve Sakoman May 16, 2023, 9:45 p.m. UTC | #1
I'm getting patch fuzz errors:

WARNING: git-2.24.4-r0 do_patch: Fuzz detected:

Applying patch CVE-2023-25652.patch
patching file apply.c
Hunk #1 succeeded at 4531 (offset 27 lines).
Hunk #2 succeeded at 4571 (offset 27 lines).
patching file t/t4115-apply-symlink.sh
Hunk #1 succeeded at 125 with fuzz 1 (offset 81 lines).

The context lines in the patches can be updated with devtool:

    devtool modify git
    devtool finish --force-patch-refresh git <layer_path>

Don't forget to review changes done by devtool!

WARNING: git-2.24.4-r0 do_patch: QA Issue: Patch log indicates that
patches do not apply cleanly. [patch-fuzz]

Steve

On Sun, May 14, 2023 at 7:25 PM Hitendra Prajapati
<hprajapati@mvista.com> wrote:
>
> Git is a revision control system. Prior to versions 2.30.9, 2.31.8, 2.32.7,
> 2.33.8, 2.34.8, 2.35.8, 2.36.6, 2.37.7, 2.38.5, 2.39.3, and 2.40.1, by feeding
> specially crafted input to `git apply --reject`, a path outside the working
> tree can be overwritten with partially controlled contents (corresponding to
> the rejected hunk(s) from the given patch). A fix is available in versions
> 2.30.9, 2.31.8, 2.32.7, 2.33.8, 2.34.8, 2.35.8, 2.36.6, 2.37.7, 2.38.5, 2.39.3,
> and 2.40.1. As a workaround, avoid using `git apply` with `--reject` when applying
> patches from an untrusted source. Use `git apply --stat` to inspect a patch before
> applying; avoid applying one that create a conflict where a link corresponding to
> the `*.rej` file exists.
>
> References:
> https://nvd.nist.gov/vuln/detail/CVE-2023-25652
>
> Upstream-Status: Backport from https://github.com/git/git/commit/9db05711c98efc14f414d4c87135a34c13586e0b
>
> Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
> ---
>  .../git/files/CVE-2023-25652.patch            | 95 +++++++++++++++++++
>  meta/recipes-devtools/git/git.inc             |  1 +
>  2 files changed, 96 insertions(+)
>  create mode 100644 meta/recipes-devtools/git/files/CVE-2023-25652.patch
>
> diff --git a/meta/recipes-devtools/git/files/CVE-2023-25652.patch b/meta/recipes-devtools/git/files/CVE-2023-25652.patch
> new file mode 100644
> index 0000000000..9dde2626cc
> --- /dev/null
> +++ b/meta/recipes-devtools/git/files/CVE-2023-25652.patch
> @@ -0,0 +1,95 @@
> +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>
> +
> +Upstream-Status: Backport [https://github.com/git/git/commit/9db05711c98efc14f414d4c87135a34c13586e0b]
> +CVE: CVE-2023-25652
> +
> +Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
> +---
> + 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 f8a046a..8253173 100644
> +--- a/apply.c
> ++++ b/apply.c
> +@@ -4504,7 +4504,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) {
> +@@ -4544,7 +4544,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 872fcda..1e9e006 100755
> +--- a/t/t4115-apply-symlink.sh
> ++++ b/t/t4115-apply-symlink.sh
> +@@ -44,4 +44,19 @@ test_expect_success 'apply --index symlink patch' '
> +
> + '
> +
> ++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.25.1
> +
> diff --git a/meta/recipes-devtools/git/git.inc b/meta/recipes-devtools/git/git.inc
> index 36318eed20..33da20cd26 100644
> --- a/meta/recipes-devtools/git/git.inc
> +++ b/meta/recipes-devtools/git/git.inc
> @@ -28,6 +28,7 @@ SRC_URI = "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \
>             file://CVE-2023-22490-2.patch \
>             file://CVE-2023-22490-3.patch \
>             file://CVE-2023-23946.patch \
> +           file://CVE-2023-25652.patch \
>             "
>  S = "${WORKDIR}/git-${PV}"
>
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#181228): https://lists.openembedded.org/g/openembedded-core/message/181228
> Mute This Topic: https://lists.openembedded.org/mt/98897685/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/recipes-devtools/git/files/CVE-2023-25652.patch b/meta/recipes-devtools/git/files/CVE-2023-25652.patch
new file mode 100644
index 0000000000..9dde2626cc
--- /dev/null
+++ b/meta/recipes-devtools/git/files/CVE-2023-25652.patch
@@ -0,0 +1,95 @@ 
+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>
+
+Upstream-Status: Backport [https://github.com/git/git/commit/9db05711c98efc14f414d4c87135a34c13586e0b]
+CVE: CVE-2023-25652
+
+Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
+---
+ 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 f8a046a..8253173 100644
+--- a/apply.c
++++ b/apply.c
+@@ -4504,7 +4504,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) {
+@@ -4544,7 +4544,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 872fcda..1e9e006 100755
+--- a/t/t4115-apply-symlink.sh
++++ b/t/t4115-apply-symlink.sh
+@@ -44,4 +44,19 @@ test_expect_success 'apply --index symlink patch' '
+ 
+ '
+ 
++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.25.1
+
diff --git a/meta/recipes-devtools/git/git.inc b/meta/recipes-devtools/git/git.inc
index 36318eed20..33da20cd26 100644
--- a/meta/recipes-devtools/git/git.inc
+++ b/meta/recipes-devtools/git/git.inc
@@ -28,6 +28,7 @@  SRC_URI = "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \
            file://CVE-2023-22490-2.patch \
            file://CVE-2023-22490-3.patch \
            file://CVE-2023-23946.patch \
+           file://CVE-2023-25652.patch \
            "
 S = "${WORKDIR}/git-${PV}"