diff options
| author | Johannes Schindelin <johannes.schindelin@gmx.de> | 2017-04-13 21:21:58 +0200 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2017-04-14 02:53:08 +0200 |
| commit | 882add136fa8319832ef373b8797ef58edb80efc (patch) | |
| tree | e36f9c09c1461ff5c06e047946f4eac680184413 /builtin/difftool.c | |
| parent | difftool: avoid strcpy (diff) | |
| download | git-882add136fa8319832ef373b8797ef58edb80efc.tar.xz git-882add136fa8319832ef373b8797ef58edb80efc.zip | |
difftool: fix use-after-free
The left and right base directories were pointed to the buf field of
two strbufs, which were subject to change.
A contrived test case shows the problem where a file with a long enough
name to force the strbuf to grow is up-to-date (hence the code path is
used where the work tree's version of the file is reused), and then a
file that is not up-to-date needs to be written (hence the code path is
used where checkout_entry() uses the previously recorded base_dir that
is invalid by now).
Let's just copy the base_dir strings for use with checkout_entry(),
never touch them until the end, and release them then. This is an easily
verifiable fix (as opposed to the next-obvious alternative: to re-set
base_dir after every loop iteration).
This fixes https://github.com/git-for-windows/git/issues/1124
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/difftool.c')
| -rw-r--r-- | builtin/difftool.c | 7 |
1 files changed, 5 insertions, 2 deletions
diff --git a/builtin/difftool.c b/builtin/difftool.c index b350b3d397..1354d0e462 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -318,6 +318,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT; struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT; struct strbuf wtdir = STRBUF_INIT; + char *lbase_dir, *rbase_dir; size_t ldir_len, rdir_len, wtdir_len; const char *workdir, *tmp; int ret = 0, i; @@ -351,11 +352,11 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, memset(&wtindex, 0, sizeof(wtindex)); memset(&lstate, 0, sizeof(lstate)); - lstate.base_dir = ldir.buf; + lstate.base_dir = lbase_dir = xstrdup(ldir.buf); lstate.base_dir_len = ldir.len; lstate.force = 1; memset(&rstate, 0, sizeof(rstate)); - rstate.base_dir = rdir.buf; + rstate.base_dir = rbase_dir = xstrdup(rdir.buf); rstate.base_dir_len = rdir.len; rstate.force = 1; @@ -625,6 +626,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, exit_cleanup(tmpdir, rc); finish: + free(lbase_dir); + free(rbase_dir); strbuf_release(&ldir); strbuf_release(&rdir); strbuf_release(&wtdir); |