From 1c10b8e5b0819598b42c042754c7c860a8637ce3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 22 Mar 2024 05:59:51 -0400 Subject: [PATCH] format-patch: fix leak of empty header string The log_write_email_headers() function recently learned to return the "extra_headers_p" variable to the caller as an allocated string. We start by copying rev_info.extra_headers into a strbuf, and then detach the strbuf at the end of the function. If there are no extra headers, we leave the strbuf empty. Likewise, if there are no headers to return, we pass back NULL. This misses a corner case which can cause a leak. The "do we have any headers to copy" check is done by looking for a NULL opt->extra_headers. But the "do we have a non-empty string to return" check is done by checking the length of the strbuf. That means if opt->extra_headers is the empty string, we'll "copy" it into the strbuf, triggering an allocation, but then leak the buffer when we return NULL from the function. We can solve this in one of two ways: 1. Rather than checking headers->len at the end, we could check headers->alloc to see if we allocated anything. That retains the original behavior before the recent change, where an empty extra_headers string is "passed through" to the caller. In practice this doesn't matter, though (the code which eventually looks at the result treats NULL or the empty string the same). 2. Only bother copying a non-empty string into the strbuf. This has the added bonus of avoiding a pointless allocation. Arguably strbuf_addstr() could do this optimization itself, though it may be slightly dangerous to do so (some existing callers may not get a fresh allocation when they expect to). In theory callers are all supposed to use strbuf_detach() in such a case, but there's no guarantee that this is the case. This patch uses option 2. Without it, building with SANITIZE=leak shows many errors in t4021 and elsewhere. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- log-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index a102893621..7e65fd7026 100644 --- a/log-tree.c +++ b/log-tree.c @@ -480,7 +480,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, *need_8bit_cte_p = 0; /* unknown */ - if (opt->extra_headers) + if (opt->extra_headers && *opt->extra_headers) strbuf_addstr(&headers, opt->extra_headers); fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name); -- 2.11.4.GIT