Merge branch 'tb/config-copy-or-rename-in-file-injection'

Avoids issues with renaming or deleting sections with long lines, where
configuration values may be interpreted as sections, leading to
configuration injection. Addresses CVE-2023-29007.

* tb/config-copy-or-rename-in-file-injection:
  config.c: disallow overly-long lines in `copy_or_rename_section_in_file()`
  config.c: avoid integer truncation in `copy_or_rename_section_in_file()`
  config: avoid fixed-sized buffer when renaming/deleting a section
  t1300: demonstrate failure when renaming sections with long lines

Signed-off-by: Taylor Blau <[email protected]>

Author: ttaylorr

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++) {
@@ -3082,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,
@@ -3091,11 +3094,12 @@ 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;
 	struct config_store_data store;
+	uint32_t line_nr = 0;
 
 	memset(&store, 0, sizeof(store));
 
@@ -3132,16 +3136,25 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
 		goto out;
 	}
 
-	while (fgets(buf, sizeof(buf), config_file)) {
-		unsigned i;
-		int length;
+	while (!strbuf_getwholeline(&buf, config_file, '\n')) {
+		size_t i, length;
 		int is_section = 0;
-		char *output = buf;
-		for (i = 0; buf[i] && isspace(buf[i]); i++)
+		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[i] == '[') {
+		if (buf.buf[i] == '[') {
 			/* it's a section */
-			int offset;
+			size_t offset;
 			is_section = 1;
 
 			/*
@@ -3158,7 +3171,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 == NULL) {
@@ -3233,6 +3246,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;
 }
 

t/t1300-config.sh

@@ -613,6 +613,36 @@ test_expect_success 'renaming to bogus section is rejected' '
 	test_must_fail git config --rename-section branch.zwei "bogus name"
 '
 
+test_expect_success '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_success '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
+'
+
+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