git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 05/11] config: use git_config_string_dup() to fix leaky open coding

From
Jeff King <peff@peff.net>
Date
Apr 7, 2024, 01:01 UTC
Message-ID
<20240407010143.GE868358@coredump.intra.peff.net>
In-Reply-To
<20240407005656.GA436890@coredump.intra.peff.net>

These are cases which open-code the equivalent of git_config_string(), but end up leaking the resulting value if the config value is seen multiple times (because they never free an existing value). Using git_config_string_dup() plug these potential leaks.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/help.c | 7 ++-----
 config.c       | 8 ++------
 merge-ll.c     | 5 +----
 3 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/builtin/help.c b/builtin/help.c
index dc1fbe2b98..1bdd2faee0 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -52,7 +52,7 @@ static enum help_action {
 	HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION,
 } cmd_mode;
 
-static const char *html_path;
+static char *html_path;
 static int verbose = 1;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
@@ -407,10 +407,7 @@ static int git_help_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "help.htmlpath")) {
-		if (!value)
-			return config_error_nonbool(var);
-		html_path = xstrdup(value);
-		return 0;
+		return git_config_string_dup(&html_path, var, value);
 	}
 	if (!strcmp(var, "man.viewer")) {
 		if (!value)
diff --git a/config.c b/config.c
index a0aa45abd5..c115e6d8c9 100644
--- a/config.c
+++ b/config.c
@@ -1575,12 +1575,8 @@ static int git_default_core_config(const char *var, const char *value,
 	if (!strcmp(var, "core.checkroundtripencoding"))
 		return git_config_string(&check_roundtrip_encoding, var, value);
 
-	if (!strcmp(var, "core.notesref")) {
-		if (!value)
-			return config_error_nonbool(var);
-		notes_ref_name = xstrdup(value);
-		return 0;
-	}
+	if (!strcmp(var, "core.notesref"))
+		return git_config_string_dup(&notes_ref_name, var, value);
 
 	if (!strcmp(var, "core.editor"))
 		return git_config_string(&editor_program, var, value);
diff --git a/merge-ll.c b/merge-ll.c
index bf1077ae09..660d9a3bd6 100644
--- a/merge-ll.c
+++ b/merge-ll.c
@@ -308,8 +308,6 @@ static int read_merge_config(const char *var, const char *value,
 		return git_config_string(&fn->description, var, value);
 
 	if (!strcmp("driver", key)) {
-		if (!value)
-			return config_error_nonbool(var);
 		/*
 		 * merge.<name>.driver specifies the command line:
 		 *
@@ -333,8 +331,7 @@ static int read_merge_config(const char *var, const char *value,
 		 * file named by %A, and signal that it has done with zero exit
 		 * status.
 		 */
-		fn->cmdline = xstrdup(value);
-		return 0;
+		return git_config_string_dup(&fn->cmdline, var, value);
 	}
 
 	if (!strcmp("recursive", key))
-- 
2.44.0.872.g288abe5b5b
Previous: Jeff KingNext: Jeff King
Message 10 of 23 in “config: do not leak excludes_file”
  1. config: do not leak excludes_fileJunio C Hamano, Apr 6, 2024
  2. [WIP] git_config_pathname() leakfixJunio C Hamano, Apr 6, 2024
  3. 0/12 git_config_string() considered harmfulJeff King, Apr 7, 2024
  4. 01/11 config: make sequencer.c's git_config_string_dup() publicJeff King, Apr 7, 2024
  5. 02/11 config: add git_config_pathname_dup()Jeff King, Apr 7, 2024
  6. 03/11 config: prefer git_config_string_dup() for temp variablesJeff King, Apr 7, 2024
  7. Eric SunshineApr 7, 2024
  8. Jeff KingApr 7, 2024
  9. 04/11 config: use git_config_string_dup() for open-coded equivalentsJeff King, Apr 7, 2024
  10. 05/11 config: use git_config_string_dup() to fix leaky open codingJeff King, Apr 7, 2024
  11. 06/11 config: use git_config_string() in easy casesJeff King, Apr 7, 2024
  12. Eric SunshineApr 7, 2024
  13. Jeff KingApr 7, 2024
  14. 07/11 config: use git_config_pathname_dup() in easy casesJeff King, Apr 7, 2024
  15. 08/11 http: use git_config_string_dup()Jeff King, Apr 7, 2024
  16. 09/11 merge: use git_config_string_dup() for pull strategiesJeff King, Apr 7, 2024
  17. 10/11 userdiff: use git_config_string_dup() when we canJeff King, Apr 7, 2024
  18. 11/11 blame: use "dup" string_list for ignore-revs filesJeff King, Apr 7, 2024
  19. Eric SunshineApr 7, 2024
  20. Jeff KingApr 7, 2024
  21. Rubén JustoApr 7, 2024
  22. Jeff KingApr 8, 2024
  23. Rubén JustoApr 11, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.