git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Taylor Blau" <me@ttaylorr.com>,
	"Carlos Andrés Ramírez Cataño" <antaigroupltda@gmail.com>
Subject: Re: [PATCH 0/2] avoiding recursion in mailinfo
Date: Fri, 15 Dec 2023 08:58:52 +0100	[thread overview]
Message-ID: <ZXwHPL3sWhzwamrp@tanuki> (raw)
In-Reply-To: <20231214214444.GB2297853@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

On Thu, Dec 14, 2023 at 04:44:44PM -0500, Jeff King wrote:
> On Thu, Dec 14, 2023 at 08:41:20AM +0100, Patrick Steinhardt wrote:
> 
> > > @@ -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;
> > >  			}
> > >  		}
> > >  
> > > I doubt it's a big deal either way, but if it's that easy to do it might
> > > be worth it.
> > 
> > Isn't this only protecting against unbalanced braces? That might be a
> > sensible check to do regardless, but does it protect against recursion
> > blowing the stack if you just happen to have many opening braces?
> > 
> > Might be I'm missing something.
> 
> It protects against recrusion blowing the stack because it removes the
> recursive call to unquote_comment(). ;)

Doh. Of course it does, I completely missed the removals.

> The "depth" stuff is there because without recursion, we have to know
> when we've hit the correct closing paren (as opposed to an embedded
> one).

Yes, makes sense now. The patches look good to me, thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2023-12-15  7:58 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         ` [PATCH 2/2] mailinfo: avoid recursion when unquoting From headers Jeff King
2023-12-15  7:58         ` Patrick Steinhardt [this message]
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=ZXwHPL3sWhzwamrp@tanuki \
    --to=ps@pks.im \
    --cc=antaigroupltda@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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).