On Thu, Dec 14, 2023 at 08:41:20AM +0100, Patrick Steinhardt wrote:

quoted

@@ -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