[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Bug-tar] [PATCH] --one-top-level: avoid a heap-buffer-overflow
From: |
Jim Meyering |
Subject: |
[Bug-tar] [PATCH] --one-top-level: avoid a heap-buffer-overflow |
Date: |
Sat, 07 Apr 2018 09:39:40 -0700 |
I configured as usual, then ran ASAN-enabled tests like this:
make CFLAGS='-O0 -ggdb3' AM_CFLAGS=-fsanitize=address \
AM_LDFLAGS='-fsanitize=address -static-libasan' check \
TESTSUITEFLAGS=--jobs=40
That exposed a buffer-overrun bug.
Here's a stand-alone reproducer using valgrind:
$ : > k && tar cf k.tar k; valgrind tar --one-top-level -tf k.tar
Invalid read of size 1
at 0x421FCF: strip_compression_suffix (suffix.c:107)
by 0x405903: decode_options (tar.c:2545)
by 0x405903: main (tar.c:2708)
Address 0x5ecfb6d is 3 bytes before a block of size 6 alloc'd
at 0x4C2CB6B: malloc (vg_replace_malloc.c:299)
by 0x52F07B9: strndup (strndup.c:43)
by 0x441948: xstrndup (xstrndup.c:32)
by 0x4058F8: decode_options (tar.c:2544)
by 0x4058F8: main (tar.c:2708)
>From e68608dd3ae21fbdb24caeddc2bba34c265f53b7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 7 Apr 2018 08:41:46 -0700
Subject: [PATCH] --one-top-level: avoid a heap-buffer-overflow
* NEWS: Mention this.
* src/suffix.c (strip_compression_suffix): Fix computation
of result string length. Also rename the now-outer "len"
to "base_len" to make clear it is the suffix-trimmed length.
Without this change, some ASAN-enabled test runs would fail
with this:
==30815==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6020000002ed at pc 0x00000049d1f4 bp 0x7ffeb5906d50 sp 0x7ffeb5906500
READ of size 1 at 0x6020000002ed thread T0
#0 0x49d1f3 in __interceptor_strncmp
/j/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:407
#1 0x5670f3 in strip_compression_suffix /j/tar/src/suffix.c:107
#2 0x575788 in decode_options /j/tar/src/tar.c:2545
#3 0x5760c0 in main /j/tar/src/tar.c:2708
#4 0x7f105090df29 in __libc_start_main ../csu/libc-start.c:308
#5 0x408629 in _start (/j/tar/src/tar+0x408629)
0x6020000002ed is located 3 bytes to the left of 6-byte region
[0x6020000002f0,0x6020000002f6)
allocated by thread T0 here:
#0 0x4d0710 in __interceptor_malloc
/j/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
#1 0x4908ad in __interceptor_strndup
/j/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:326
#2 0x5cbcbd in xstrndup /j/tar/gnu/xstrndup.c:32
#3 0x5a325b in base_name /j/tar/gnu/basename.c:57
#4 0x575772 in decode_options /j/tar/src/tar.c:2544
#5 0x5760c0 in main /j/tar/src/tar.c:2708
#6 0x7f105090df29 in __libc_start_main ../csu/libc-start.c:308
---
NEWS | 7 +++++--
src/suffix.c | 11 +++++------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/NEWS b/NEWS
index 998258e..ee607ed 100644
--- a/NEWS
+++ b/NEWS
@@ -1,9 +1,12 @@
-GNU tar NEWS - User visible changes. 2018-03-18
+GNU tar NEWS - User visible changes. 2018-04-07
Please send GNU tar bug reports to <address@hidden>
version 1.30.90 (Git)
+* Fix heap-buffer-overrun with --one-top-level.
+Bug introduced with the addition of that option in 1.28.
+
* Support for zstd compression
New option '--zstd' instructs tar to use zstd as compression program.
@@ -53,7 +56,7 @@ causing subsequent link extractions in that directory to fail.
This new warning control option suppresses warning messages about
unreadable files and directories. It has effect only if used together
-with the --ignore-failed-read option.
+with the --ignore-failed-read option.
* The --warnings=none option now suppresses all warnings
diff --git a/src/suffix.c b/src/suffix.c
index 66b5694..72872b0 100644
--- a/src/suffix.c
+++ b/src/suffix.c
@@ -62,7 +62,7 @@ find_compression_suffix (const char *name, size_t *ret_len)
{
size_t len;
struct compression_suffix *p;
-
+
suf++;
len = strlen (suf);
@@ -100,12 +100,12 @@ char *
strip_compression_suffix (const char *name)
{
char *s = NULL;
- size_t len;
+ size_t base_len;
- if (find_compression_suffix (name, &len))
+ if (find_compression_suffix (name, &base_len))
{
- if (strncmp (name + len - 4, ".tar", 4) == 0)
- len -= 4;
+ size_t len = (strncmp (name + base_len, ".tar", 4) == 0
+ ? base_len : strlen (name));
if (len == 0)
return NULL;
s = xmalloc (len + 1);
@@ -114,4 +114,3 @@ strip_compression_suffix (const char *name)
}
return s;
}
-
--
2.17.0
- [Bug-tar] [PATCH] --one-top-level: avoid a heap-buffer-overflow,
Jim Meyering <=