Merge branch 'ps/bundle-uri-arbitrary-writes' into maint-2.43

This merges in the fix for CVE-2025-48385.

* ps/bundle-uri-arbitrary-writes:
  bundle-uri: fix arbitrary file writes via parameter injection

Signed-off-by: Taylor Blau <me@ttaylorr.com>

2 files changed, 45 insertions, 0 deletions

diff --git a/bundle-uri.c b/bundle-uri.c
index ca32050a78..a6a3c1c4b3 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -292,6 +292,28 @@ static int download_https_uri_to_file(const char *file, const char *uri)
 	struct strbuf line = STRBUF_INIT;
 	int found_get = 0;
 
+	/*
+	 * The protocol we speak with git-remote-https(1) uses a space to
+	 * separate between URI and file, so the URI itself must not contain a
+	 * space. If it did, an adversary could change the location where the
+	 * downloaded file is being written to.
+	 *
+	 * Similarly, we use newlines to separate commands from one another.
+	 * Consequently, neither the URI nor the file must contain a newline or
+	 * otherwise an adversary could inject arbitrary commands.
+	 *
+	 * TODO: Restricting newlines in the target paths may break valid
+	 *       usecases, even if those are a bit more on the esoteric side.
+	 *       If this ever becomes a problem we should probably think about
+	 *       alternatives. One alternative could be to use NUL-delimited
+	 *       requests in git-remote-http(1). Another alternative could be
+	 *       to use URL quoting.
+	 */
+	if (strpbrk(uri, " \n"))
+		return error("bundle-uri: URI is malformed: '%s'", file);
+	if (strchr(file, '\n'))
+		return error("bundle-uri: filename is malformed: '%s'", file);
+
 	strvec_pushl(&cp.args, "git-remote-https", uri, NULL);
 	cp.err = -1;
 	cp.in = -1;
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index 996a08e90c..2af523aaa4 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -1052,6 +1052,29 @@ test_expect_success 'bundles are downloaded once during fetch --all' '
 		trace-mult.txt >bundle-fetches &&
 	test_line_count = 1 bundle-fetches
 '
+
+test_expect_success 'bundles with space in URI are rejected' '
+	test_when_finished "rm -rf busted repo" &&
+	mkdir -p "$HOME/busted/ /$HOME/repo/.git/objects/bundles" &&
+	git clone --bundle-uri="$HTTPD_URL/bogus $HOME/busted/" "$HTTPD_URL/smart/fetch.git" repo 2>err &&
+	test_grep "error: bundle-uri: URI is malformed: " err &&
+	find busted -type f >files &&
+	test_must_be_empty files
+'
+
+test_expect_success 'bundles with newline in URI are rejected' '
+	test_when_finished "rm -rf busted repo" &&
+	git clone --bundle-uri="$HTTPD_URL/bogus\nget $HTTPD_URL/bogus $HOME/busted" "$HTTPD_URL/smart/fetch.git" repo 2>err &&
+	test_grep "error: bundle-uri: URI is malformed: " err &&
+	test_path_is_missing "$HOME/busted"
+'
+
+test_expect_success 'bundles with newline in target path are rejected' '
+	git clone --bundle-uri="$HTTPD_URL/bogus" "$HTTPD_URL/smart/fetch.git" "$(printf "escape\nget $HTTPD_URL/bogus .")" 2>err &&
+	test_grep "error: bundle-uri: filename is malformed: " err &&
+	test_path_is_missing escape
+'
+
 # Do not add tests here unless they use the HTTP server, as they will
 # not run unless the HTTP dependencies exist.