[PATCH 0/2] avoiding recursion in mailinfo
- From
Jeff King <peff@peff.net>
- Date
- Dec 14, 2023, 21:44 UTC
- Message-ID
- <20231214214444.GB2297853@coredump.intra.peff.net>
- In-Reply-To
- <ZXqxoKLFG19tMFpF@tanuki>
On Thu, Dec 14, 2023 at 08:41:20AM +0100, Patrick Steinhardt wrote:
Show 25 quoted lines
> > @@ -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(). ;)
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).
Here it is as a patch (actually two patches). I don't think it's high priority, but I'd sunk enough brain cells into thinking about it that I wanted to capture the work. ;)
I did it on top of the earlier mailinfo out-of-bounds read fix, but it could be applied separately.
[1/2]: t5100: make rfc822 comment test more careful [2/2]: mailinfo: avoid recursion when unquoting From headers
mailinfo.c | 8 ++++++-- t/t5100/comment.expect | 2 +- t/t5100/comment.in | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-)
-Peff