summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar Thomas Rast <trast@inf.ethz.ch>2013-04-30 14:53:06 +0200
committerLibravatar Junio C Hamano <gitster@pobox.com>2013-04-30 15:43:48 -0700
commit756a042600a3e234e7f1eae92c84d395c6833c6d (patch)
tree6d330e2e7de3dcf576d466e67a91851bae74a437
parentsha1_file: remove recursion in unpack_entry (diff)
downloadtgif-756a042600a3e234e7f1eae92c84d395c6833c6d.tar.xz
unpack_entry: avoid freeing objects in base cache
In the !delta_data error path of unpack_entry(), we run free(base). This became a window for use-after-free() in abe601b (sha1_file: remove recursion in unpack_entry, 2013-03-27), as follows: Before abe601b, we got the 'base' from cache_or_unpack_entry(..., 0); keep_cache=0 tells it to also remove that entry. So the 'base' is at this point not cached, and freeing it in the error path is the right thing. After abe601b, the structure changed: we use a three-phase approach where phase 1 finds the innermost base or a base that is already in the cache. In phase 3 we therefore know that all bases we unpack are not part of the delta cache yet. (Observe that we pop from the cache in phase 1, so this is also true for the very first base.) So we make no further attempts to look up the bases in the cache, and just call add_delta_base_cache() on every base object we have assembled. But the !delta_data error path remained unchanged, and now calls free() on a base that has already been entered in the cache. This means that there is a use-after-free if we later use the same base again. So remove that free(); we are still going to use that data. Reported-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--sha1_file.c1
1 files changed, 0 insertions, 1 deletions
diff --git a/sha1_file.c b/sha1_file.c
index b220a470f9..a016a699f9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2040,7 +2040,6 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
error("failed to unpack compressed delta "
"at offset %"PRIuMAX" from %s",
(uintmax_t)curpos, p->pack_name);
- free(base);
data = NULL;
continue;
}