Skip to content

Commit

Permalink
mailinfo: avoid recursion when unquoting From headers
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
peff authored and gitster committed Dec 14, 2023
1 parent 2d9396c commit dee1829
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions mailinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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, '(');

Expand All @@ -70,11 +71,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;
}
}

Expand Down

0 comments on commit dee1829

Please sign in to comment.