git: Security fix for CVE-2023-22490 and CVE-2023-23946

Upstream-Status: Backport from https://github.com/git/git/commit/58325b93c5b6212697b088371809e9948fee8052 & https://github.com/git/git/commit/cf8f6ce02a13f4d1979a53241afbee15a293fce9 & https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c (From OE-Core rev: 071fb3b177bcbdd02ae2c28aad97af681c091e42) Signed-off-by: Vijay Anusuri <vanusuri@mvista.com> Signed-off-by: Steve Sakoman <steve@sakoman.com>
2026-07-21 05:27:01 +00:00 · 2023-03-20 14:41:51 +05:30
parent 8ed92cf2b3
commit 8064d53745
5 changed files with 643 additions and 0 deletions
@@ -0,0 +1,122 @@
+From cf8f6ce02a13f4d1979a53241afbee15a293fce9 Mon Sep 17 00:00:00 2001
+From: Taylor Blau <me@ttaylorr.com>
+Date: Tue, 24 Jan 2023 19:43:48 -0500
+Subject: [PATCH 2/3] clone: delay picking a transport until after get_repo_path()
+
+In the previous commit, t5619 demonstrates an issue where two calls to
+`get_repo_path()` could trick Git into using its local clone mechanism
+in conjunction with a non-local transport.
+
+That sequence is:
+
+ - the starting state is that the local path https:/example.com/foo is a
+   symlink that points to ../../../.git/modules/foo. So it's dangling.
+
+ - get_repo_path() sees that no such path exists (because it's
+   dangling), and thus we do not canonicalize it into an absolute path
+
+ - because we're using --separate-git-dir, we create .git/modules/foo.
+   Now our symlink is no longer dangling!
+
+ - we pass the url to transport_get(), which sees it as an https URL.
+
+ - we call get_repo_path() again, on the url. This second call was
+   introduced by f38aa83 (use local cloning if insteadOf makes a
+   local URL, 2014-07-17). The idea is that we want to pull the url
+   fresh from the remote.c API, because it will apply any aliases.
+
+And of course now it sees that there is a local file, which is a
+mismatch with the transport we already selected.
+
+The issue in the above sequence is calling `transport_get()` before
+deciding whether or not the repository is indeed local, and not passing
+in an absolute path if it is local.
+
+This is reminiscent of a similar bug report in [1], where it was
+suggested to perform the `insteadOf` lookup earlier. Taking that
+approach may not be as straightforward, since the intent is to store the
+original URL in the config, but to actually fetch from the insteadOf
+one, so conflating the two early on is a non-starter.
+
+Note: we pass the path returned by `get_repo_path(remote->url[0])`,
+which should be the same as `repo_name` (aside from any `insteadOf`
+rewrites).
+
+We *could* pass `absolute_pathdup()` of the same argument, which
+86521ac (Bring local clone's origin URL in line with that of a remote
+clone, 2008-09-01) indicates may differ depending on the presence of
+".git/" for a non-bare repo. That matters for forming relative submodule
+paths, but doesn't matter for the second call, since we're just feeding
+it to the transport code, which is fine either way.
+
+[1]: https://lore.kernel.org/git/CAMoD=Bi41mB3QRn3JdZL-FGHs4w3C2jGpnJB-CqSndO7FMtfzA@mail.gmail.com/
+
+Signed-off-by: Jeff King <peff@peff.net>
+Signed-off-by: Taylor Blau <me@ttaylorr.com>
+Signed-off-by: Junio C Hamano <gitster@pobox.com>
+
+Upstream-Status: Backport
+[https://github.com/git/git/commit/cf8f6ce02a13f4d1979a53241afbee15a293fce9]
+CVE: CVE-2023-22490
+Signed-off-by: Vijay Anusuri <vanusuri@mvista.com>
+---
+ builtin/clone.c                            |  8 ++++----
+ t/t5619-clone-local-ambiguous-transport.sh | 15 +++++++++++----
+ 2 files changed, 15 insertions(+), 8 deletions(-)
+
+diff --git a/builtin/clone.c b/builtin/clone.c
+index 53e04b1..b57e703 100644
+--- a/builtin/clone.c
+++ b/builtin/clone.c
+@@ -1112,10 +1112,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
+ 		    branch_top.buf);
+ 	refspec_append(&remote->fetch, default_refspec.buf);
+ 
+-	transport = transport_get(remote, remote->url[0]);
+-	transport_set_verbosity(transport, option_verbosity, option_progress);
+-	transport->family = family;
+-
+ 	path = get_repo_path(remote->url[0], &is_bundle);
+ 	is_local = option_local != 0 && path && !is_bundle;
+ 	if (is_local) {
+@@ -1135,6 +1131,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
+ 	}
+ 	if (option_local > 0 && !is_local)
+ 		warning(_("--local is ignored"));
+
+	transport = transport_get(remote, path ? path : remote->url[0]);
+	transport_set_verbosity(transport, option_verbosity, option_progress);
+	transport->family = family;
+ 	transport->cloning = 1;
+ 
+ 	transport_set_option(transport, TRANS_OPT_KEEP, "yes");
+diff --git a/t/t5619-clone-local-ambiguous-transport.sh b/t/t5619-clone-local-ambiguous-transport.sh
+index 7ebd31a..cce62bf 100644
+--- a/t/t5619-clone-local-ambiguous-transport.sh
+++ b/t/t5619-clone-local-ambiguous-transport.sh
+@@ -53,11 +53,18 @@ test_expect_success 'setup' '
+ 	git -C "$REPO" update-server-info
+ '
+ 
+-test_expect_failure 'ambiguous transport does not lead to arbitrary file-inclusion' '
+test_expect_success 'ambiguous transport does not lead to arbitrary file-inclusion' '
+ 	git clone malicious clone &&
+-	git -C clone submodule update --init &&
+-
+-	test_path_is_missing clone/.git/modules/sub/objects/secret
+	test_must_fail git -C clone submodule update --init 2>err &&
+
+	test_path_is_missing clone/.git/modules/sub/objects/secret &&
+	# We would actually expect "transport .file. not allowed" here,
+	# but due to quirks of the URL detection in Git, we mis-parse
+	# the absolute path as a bogus URL and die before that step.
+	#
+	# This works for now, and if we ever fix the URL detection, it
+	# is OK to change this to detect the transport error.
+	grep "protocol .* is not supported" err
+ '
+ 
+ test_done
+-- 
+2.25.1
+