git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, "Taylor Blau" <me@ttaylorr.com>,
	"Carlos Andrés Ramírez Cataño" <antaigroupltda@gmail.com>
Subject: [PATCH 2/2] mailinfo: avoid recursion when unquoting From headers
Date: Thu, 14 Dec 2023 16:48:59 -0500	[thread overview]
Message-ID: <20231214214859.GB2798346@coredump.intra.peff.net> (raw)
In-Reply-To: <20231214214444.GB2297853@coredump.intra.peff.net>

Our unquote_comment() function is recursive; when it sees a comment
within a comment, like:

  (this is an (embedded) comment)

it recurses to handle the inner comment. This is fine for practical use,
but it does mean that you can easily run out of stack space with a
malicious header. For example:

  perl -e 'print "From: ", "(" x 2**18;' |
  git mailinfo /dev/null /dev/null

segfaults on my system. And since mailinfo is likely to be fed untrusted
input from the Internet (if not by human users, who might recognize a
garbage header, but certainly there are automated systems that apply
patches from a list) it may be possible for an attacker to trigger the
problem.

That said, I don't think there's an interesting security vulnerability
here. All an attacker can do is make it impossible to parse their email
and apply their patch, and there are lots of ways to generate bogus
emails. So it's more of an annoyance than anything.

But it's pretty easy to fix it. The recursion is not helping us preserve
any particular state from each level. The only flag in our parsing is
take_next_literally, and we can never recurse when it is set (since the
start of a new comment implies it was not backslash-escaped). So it is
really only useful for finding the end of the matched pair of
parentheses. We can do that easily with a simple depth counter.

Signed-off-by: Jeff King <peff@peff.net>
---
 mailinfo.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mailinfo.c b/mailinfo.c
index 737b9e5e13..db236f9f9f 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -59,6 +59,7 @@ static void parse_bogus_from(struct mailinfo *mi, const struct strbuf *line)
 static const char *unquote_comment(struct strbuf *outbuf, const char *in)
 {
 	int take_next_literally = 0;
+	int depth = 1;
 
 	strbuf_addch(outbuf, '(');
 
@@ -72,11 +73,14 @@ static const char *unquote_comment(struct strbuf *outbuf, const char *in)
 				take_next_literally = 1;
 				continue;
 			case '(':
-				in = unquote_comment(outbuf, in);
+				strbuf_addch(outbuf, '(');
+				depth++;
 				continue;
 			case ')':
 				strbuf_addch(outbuf, ')');
-				return in;
+				if (!--depth)
+					return in;
+				continue;
 			}
 		}
 
-- 
2.43.0.363.g1d22a8f302

  parent reply	other threads:[~2023-12-14 21:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12 22:12 [PATCH] mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair() Jeff King
2023-12-13  7:07 ` Patrick Steinhardt
2023-12-13  8:20   ` Jeff King
2023-12-14  7:41     ` Patrick Steinhardt
2023-12-14 21:44       ` [PATCH 0/2] avoiding recursion in mailinfo Jeff King
2023-12-14 21:47         ` [PATCH 1/2] t5100: make rfc822 comment test more careful Jeff King
2023-12-14 21:48         ` Jeff King [this message]
2023-12-15  7:58         ` [PATCH 0/2] avoiding recursion in mailinfo Patrick Steinhardt
2023-12-13 14:54   ` [PATCH] mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair() Junio C Hamano
2023-12-14 21:40     ` Jeff King

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=20231214214859.GB2798346@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=antaigroupltda@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=ps@pks.im \
    /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).