From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 05 Apr 2013 23:49:15 GMT
Subject: Re: [PATCH] show-branch: use strbuf instead of static buffer
Message-ID: <20130405234915.GU30308@google.com>
In-Reply-To: <20130405211550.GA4880@sigill.intra.peff.net>

Jeff King wrote:

> When we generate relative names (e.g., "master~20^2"), we
> format the name into a static buffer, then xstrdup the
> result to attach it to the commit. Since the first thing we
> add into the static buffer is the already-computed name of
> the child commit, the names may get longer and longer as
> the traversal gets deeper, and we may eventually overflow
> the fixed-size buffer.

Good catch.

[...]
> Though this is a stack overflow, I don't know that it's exploitable for
> anything interesting; an attacker does not get to write arbitrary data,
> but rather only a sequence of "^%d" and "~%d" relative history markers.
> Perhaps in theory one could devise a history such that the sequence
> markers spelled out some malicious code, but it would be quite a
> challenge

Overwrite the return address and return-to-libc?

[...]
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c

Very clean and obviously correct.  Thanks.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

A test would be nice, though.

Hope that helps,
Jonathan