path: root/builtin
diff options
authorJohannes Schindelin <>2017-04-13 19:21:58 (GMT)
committerJunio C Hamano <>2017-04-14 00:53:08 (GMT)
commit882add136fa8319832ef373b8797ef58edb80efc (patch)
treee36f9c09c1461ff5c06e047946f4eac680184413 /builtin
parent0730dd4ffb39358f30b1a956cd9182aed1958b47 (diff)
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 Signed-off-by: Johannes Schindelin <> Reviewed-by: Jonathan Nieder <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin')
1 files changed, 5 insertions, 2 deletions
diff --git a/builtin/difftool.c b/builtin/difftool.c
index b350b3d..1354d0e 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);
+ free(lbase_dir);
+ free(rbase_dir);