A crafted GNUTYPE_DUMPDIR member or pax GNU.dumpdir record produces a dumpdir buffer with no terminating empty record, while consumers walk it with strlen, reading past the allocation. purge_directory then uses over-read bytes as filenames for rename/unlink.
* src/incremen.c (get_gnu_dumpdir): Allocate size + 2 and append two NUL bytes to terminate the dumpdir. * src/xheader.c (dumpdir_decoder): Likewise. Signed-off-by: Sayed Kaif <[email protected]> --- src/incremen.c | 9 ++++++++- src/xheader.c | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/incremen.c b/src/incremen.c index 53c6b3f3..ac46cf63 100644 --- a/src/incremen.c +++ b/src/incremen.c @@ -1503,7 +1503,13 @@ get_gnu_dumpdir (struct tar_stat_info *stat_info) size = stat_info->stat.st_size; - archive_dir = xmalloc (size); + /* The dumpdir is consumed as a sequence of NUL-terminated strings + ending with an empty one (see dumpdir_size, dumpdir_ok and + purge_directory), i.e. it must end with two NUL bytes. A crafted + archive need not supply that terminator; without it those + strlen-based consumers read past the end of this buffer. Allocate + two extra bytes and append the terminator below. */ + archive_dir = xmalloc (size + 2); to = archive_dir; set_next_block_after (current_header); @@ -1525,6 +1531,7 @@ get_gnu_dumpdir (struct tar_stat_info *stat_info) mv_end (); + to[0] = to[1] = '\0'; stat_info->dumpdir = archive_dir; stat_info->skipped = true; /* For skip_member() and friends to work correctly */ diff --git a/src/xheader.c b/src/xheader.c index 05f905ed..61e942f6 100644 --- a/src/xheader.c +++ b/src/xheader.c @@ -1483,8 +1483,15 @@ dumpdir_decoder (struct tar_stat_info *st, char const *arg, idx_t size) { - st->dumpdir = ximalloc (size); + /* The dumpdir is consumed as a sequence of NUL-terminated strings + ending with an empty one (see dumpdir_size, dumpdir_ok and + purge_directory), i.e. it must end with two NUL bytes. A crafted + archive may supply a value that lacks this terminator, which would + make those strlen-based walkers read past the buffer. Append the + two terminating NUL bytes so the walk always stops in bounds. */ + st->dumpdir = ximalloc (size + 2); memcpy (st->dumpdir, arg, size); + st->dumpdir[size] = st->dumpdir[size + 1] = '\0'; } static void -- 2.52.0.windows.1
