git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Lidong Yan via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Lidong Yan <502024330056@smail.nju.edu.cn>,
	Lidong Yan <502024330056@smail.nju.edu.cn>
Subject: [PATCH v2] mailinfo: fix pointential memory leak if `decode_header` failed
Date: Sun, 11 May 2025 16:14:57 +0000	[thread overview]
Message-ID: <pull.1956.v2.git.git.1746980097510.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1956.git.git.1746711521614.gitgitgadget@gmail.com>

From: Lidong Yan <502024330056@smail.nju.edu.cn>

In mailinfo.c:decode_header, if convert_to_utf8 failed, the strbuf stored
in dec will leak. Simply add strbuf_release and free(dec) will solve
this problem.

Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
---
    decode_header: fix pointential memory leak if decode_header failed
    
    In mailinfo.c line 539, if convert_to_utf8 failed, the strbuf stored in
    dec will leak. Simply add strbuf_release and free(dec) will solve this
    problem.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1956%2Fbrandb97%2Ffix-mailinfo-decode-header-leak-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1956/brandb97/fix-mailinfo-decode-header-leak-v2
Pull-Request: https://github.com/git/git/pull/1956

Range-diff vs v1:

 1:  81fdfb94315 ! 1:  90dc9b0d49b decode_header: fix pointential memory leak if decode_header failed
     @@ Metadata
      Author: Lidong Yan <502024330056@smail.nju.edu.cn>
      
       ## Commit message ##
     -    decode_header: fix pointential memory leak if decode_header failed
     +    mailinfo: fix pointential memory leak if `decode_header` failed
      
     -    In mailinfo.c line 539, if convert_to_utf8 failed, the strbuf stored
     +    In mailinfo.c:decode_header, if convert_to_utf8 failed, the strbuf stored
          in dec will leak. Simply add strbuf_release and free(dec) will solve
          this problem.
      
          Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
      
       ## mailinfo.c ##
     +@@ mailinfo.c: static int is_format_patch_separator(const char *line, int len)
     + 	return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
     + }
     + 
     +-static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
     ++static int decode_q_segment(struct strbuf *out, const struct strbuf *q_seg,
     ++			    int rfc2047)
     + {
     + 	const char *in = q_seg->buf;
     + 	int c;
     +-	struct strbuf *out = xmalloc(sizeof(struct strbuf));
     + 	strbuf_init(out, q_seg->len);
     + 
     + 	while ((c = *in++) != 0) {
     +@@ mailinfo.c: static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
     + 			c = 0x20;
     + 		strbuf_addch(out, c);
     + 	}
     +-	return out;
     ++	return 0;
     + }
     + 
     +-static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
     ++static int decode_b_segment(struct strbuf *out, const struct strbuf *b_seg)
     + {
     + 	/* Decode in..ep, possibly in-place to ot */
     + 	int c, pos = 0, acc = 0;
     + 	const char *in = b_seg->buf;
     +-	struct strbuf *out = xmalloc(sizeof(struct strbuf));
     + 	strbuf_init(out, b_seg->len);
     + 
     + 	while ((c = *in++) != 0) {
     +@@ mailinfo.c: static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
     + 			break;
     + 		}
     + 	}
     +-	return out;
     ++	return 0;
     + }
     + 
     + static int convert_to_utf8(struct mailinfo *mi,
     +@@ mailinfo.c: static int convert_to_utf8(struct mailinfo *mi,
     + static void decode_header(struct mailinfo *mi, struct strbuf *it)
     + {
     + 	char *in, *ep, *cp;
     +-	struct strbuf outbuf = STRBUF_INIT, *dec;
     ++	struct strbuf outbuf = STRBUF_INIT, dec = STRBUF_INIT;
     + 	struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
     + 	int found_error = 1; /* pessimism */
     + 
      @@ mailinfo.c: static void decode_header(struct mailinfo *mi, struct strbuf *it)
     - 			dec = decode_q_segment(&piecebuf, 1);
     + 		default:
     + 			goto release_return;
     + 		case 'b':
     +-			dec = decode_b_segment(&piecebuf);
     ++			if ((found_error = decode_b_segment(&dec, &piecebuf))) {
     ++				goto release_return;
     ++			}
     + 			break;
     + 		case 'q':
     +-			dec = decode_q_segment(&piecebuf, 1);
     ++			if ((found_error = decode_q_segment(&dec, &piecebuf, 1))) {
     ++				goto release_return;
     ++			}
       			break;
       		}
      -		if (convert_to_utf8(mi, dec, charset_q.buf))
     -+		if (convert_to_utf8(mi, dec, charset_q.buf)) {
     -+			strbuf_release(dec);
     -+			free(dec);
     ++		if (convert_to_utf8(mi, &dec, charset_q.buf)) {
     ++			strbuf_release(&dec);
       			goto release_return;
      +		}
       
     - 		strbuf_addbuf(&outbuf, dec);
     - 		strbuf_release(dec);
     +-		strbuf_addbuf(&outbuf, dec);
     +-		strbuf_release(dec);
     +-		free(dec);
     ++		strbuf_addbuf(&outbuf, &dec);
     ++		strbuf_release(&dec);
     + 		in = ep + 2;
     + 	}
     + 	strbuf_addstr(&outbuf, in);
     +@@ mailinfo.c: static int is_inbody_header(const struct mailinfo *mi,
     + 
     + static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
     + {
     +-	struct strbuf *ret;
     ++	struct strbuf ret;
     ++	int found_error = 0;
     + 
     + 	switch (mi->transfer_encoding) {
     + 	case TE_QP:
     +-		ret = decode_q_segment(line, 0);
     ++		found_error = decode_q_segment(&ret, line, 0);
     + 		break;
     + 	case TE_BASE64:
     +-		ret = decode_b_segment(line);
     ++		found_error = decode_b_segment(&ret, line);
     + 		break;
     + 	case TE_DONTCARE:
     + 	default:
     + 		return;
     + 	}
     + 	strbuf_reset(line);
     +-	strbuf_addbuf(line, ret);
     +-	strbuf_release(ret);
     +-	free(ret);
     ++	strbuf_addbuf(line, &ret);
     ++	if (!found_error)
     ++		strbuf_release(&ret);
     + }
     + 
     + static inline int patchbreak(const struct strbuf *line)


 mailinfo.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 7b001fa5dbd..11ec3914ce4 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -381,11 +381,11 @@ static int is_format_patch_separator(const char *line, int len)
 	return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
 }
 
-static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
+static int decode_q_segment(struct strbuf *out, const struct strbuf *q_seg,
+			    int rfc2047)
 {
 	const char *in = q_seg->buf;
 	int c;
-	struct strbuf *out = xmalloc(sizeof(struct strbuf));
 	strbuf_init(out, q_seg->len);
 
 	while ((c = *in++) != 0) {
@@ -405,15 +405,14 @@ static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
 			c = 0x20;
 		strbuf_addch(out, c);
 	}
-	return out;
+	return 0;
 }
 
-static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
+static int decode_b_segment(struct strbuf *out, const struct strbuf *b_seg)
 {
 	/* Decode in..ep, possibly in-place to ot */
 	int c, pos = 0, acc = 0;
 	const char *in = b_seg->buf;
-	struct strbuf *out = xmalloc(sizeof(struct strbuf));
 	strbuf_init(out, b_seg->len);
 
 	while ((c = *in++) != 0) {
@@ -447,7 +446,7 @@ static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
 			break;
 		}
 	}
-	return out;
+	return 0;
 }
 
 static int convert_to_utf8(struct mailinfo *mi,
@@ -475,7 +474,7 @@ static int convert_to_utf8(struct mailinfo *mi,
 static void decode_header(struct mailinfo *mi, struct strbuf *it)
 {
 	char *in, *ep, *cp;
-	struct strbuf outbuf = STRBUF_INIT, *dec;
+	struct strbuf outbuf = STRBUF_INIT, dec = STRBUF_INIT;
 	struct strbuf charset_q = STRBUF_INIT, piecebuf = STRBUF_INIT;
 	int found_error = 1; /* pessimism */
 
@@ -530,18 +529,23 @@ static void decode_header(struct mailinfo *mi, struct strbuf *it)
 		default:
 			goto release_return;
 		case 'b':
-			dec = decode_b_segment(&piecebuf);
+			if ((found_error = decode_b_segment(&dec, &piecebuf))) {
+				goto release_return;
+			}
 			break;
 		case 'q':
-			dec = decode_q_segment(&piecebuf, 1);
+			if ((found_error = decode_q_segment(&dec, &piecebuf, 1))) {
+				goto release_return;
+			}
 			break;
 		}
-		if (convert_to_utf8(mi, dec, charset_q.buf))
+		if (convert_to_utf8(mi, &dec, charset_q.buf)) {
+			strbuf_release(&dec);
 			goto release_return;
+		}
 
-		strbuf_addbuf(&outbuf, dec);
-		strbuf_release(dec);
-		free(dec);
+		strbuf_addbuf(&outbuf, &dec);
+		strbuf_release(&dec);
 		in = ep + 2;
 	}
 	strbuf_addstr(&outbuf, in);
@@ -634,23 +638,24 @@ static int is_inbody_header(const struct mailinfo *mi,
 
 static void decode_transfer_encoding(struct mailinfo *mi, struct strbuf *line)
 {
-	struct strbuf *ret;
+	struct strbuf ret;
+	int found_error = 0;
 
 	switch (mi->transfer_encoding) {
 	case TE_QP:
-		ret = decode_q_segment(line, 0);
+		found_error = decode_q_segment(&ret, line, 0);
 		break;
 	case TE_BASE64:
-		ret = decode_b_segment(line);
+		found_error = decode_b_segment(&ret, line);
 		break;
 	case TE_DONTCARE:
 	default:
 		return;
 	}
 	strbuf_reset(line);
-	strbuf_addbuf(line, ret);
-	strbuf_release(ret);
-	free(ret);
+	strbuf_addbuf(line, &ret);
+	if (!found_error)
+		strbuf_release(&ret);
 }
 
 static inline int patchbreak(const struct strbuf *line)

base-commit: 6f84262c44a89851c3ae5a6e4c1a9d06b2068d75
-- 
gitgitgadget

  parent reply	other threads:[~2025-05-11 16:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-08 13:38 [PATCH] decode_header: fix pointential memory leak if decode_header failed Lidong Yan via GitGitGadget
2025-05-08 22:16 ` Junio C Hamano
2025-05-09  1:51   ` lidongyan
2025-05-11 16:14 ` Lidong Yan via GitGitGadget [this message]
2025-05-12 14:12   ` [PATCH v2] mailinfo: fix pointential memory leak if `decode_header` failed Junio C Hamano
2025-05-12 16:17   ` [PATCH v3] " Lidong Yan via GitGitGadget
2025-05-12 19:32     ` Junio C Hamano
2025-05-13  2:49     ` [PATCH v4] " Lidong Yan via GitGitGadget
2025-05-13 13:34       ` Junio C Hamano
2025-05-13 13:42         ` lidongyan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1956.v2.git.git.1746980097510.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=502024330056@smail.nju.edu.cn \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).