Bug#731860: libtar: CVE-2013-4420: directory traversal when extracting archives
On 13 February 2014 19:23, Magnus Holmgren holmg...@debian.org wrote: tisdagen den 11 februari 2014 11.26.15 skrev du: On 9 February 2014 22:08, Magnus Holmgren mag...@kibibyte.se wrote: The first if should be a while, shouldn't it? Otherwise we'll only skip over the first ../ if file_name starts with ../../, if I'm not mistaken. That's handled by the while loop right after the if. Attached test case contains an entry called ../../../empty-file tar tf should print a warning message and list the full path, while libtar should simply print it as 'empty-file'. Yes, an odd number of .. will yield the desired result, but the even ..s will be missed. Ah, yes, indeed. Nice catch. Cheers, -- Raphael Geissert - Debian Developer www.debian.org - get.debian.net -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#731860: libtar: CVE-2013-4420: directory traversal when extracting archives
tisdagen den 11 februari 2014 11.26.15 skrev du: On 9 February 2014 22:08, Magnus Holmgren mag...@kibibyte.se wrote: The first if should be a while, shouldn't it? Otherwise we'll only skip over the first ../ if file_name starts with ../../, if I'm not mistaken. That's handled by the while loop right after the if. Attached test case contains an entry called ../../../empty-file tar tf should print a warning message and list the full path, while libtar should simply print it as 'empty-file'. Yes, an odd number of .. will yield the desired result, but the even ..s will be missed. We should also strip any leading slashes to be of any use, so I'll add that too. -- Magnus Holmgrenholmg...@debian.org Debian Developer signature.asc Description: This is a digitally signed message part.
Bug#731860: libtar: CVE-2013-4420: directory traversal when extracting archives
Hi, On 9 February 2014 22:08, Magnus Holmgren mag...@kibibyte.se wrote: The first if should be a while, shouldn't it? Otherwise we'll only skip over the first ../ if file_name starts with ../../, if I'm not mistaken. That's handled by the while loop right after the if. Attached test case contains an entry called ../../../empty-file tar tf should print a warning message and list the full path, while libtar should simply print it as 'empty-file'. Cheers, -- Raphael Geissert - Debian Developer www.debian.org - get.debian.net triple-double-dot.tar Description: Unix tar archive
Bug#731860: libtar: CVE-2013-4420: directory traversal when extracting archives
tisdagen den 10 december 2013 16.27.32 skrev du: CVE-2013-4420[0]: tar_extract_glob and tar_extract_all path prefix directory traversal Attached is a proposed patch that makes libtar work similarly to tar. The first if should be a while, shouldn't it? Otherwise we'll only skip over the first ../ if file_name starts with ../../, if I'm not mistaken. -- Magnus Holmgrenholmg...@debian.org Debian Developer signature.asc Description: This is a digitally signed message part.
Bug#731860: libtar: CVE-2013-4420: directory traversal when extracting archives
Source: libtar
Severity: grave
Tags: security
Hi,
the following vulnerability was published for libtar.
CVE-2013-4420[0]:
tar_extract_glob and tar_extract_all path prefix directory traversal
If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities Exposures) id in your changelog entry.
For further information see:
[0] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-4420
http://security-tracker.debian.org/tracker/CVE-2013-4420
Attached is a proposed patch that makes libtar work similarly to tar.
Cheers,
--
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net
Index: libtar-1.2.16/lib/decode.c
===
--- libtar-1.2.16.orig/lib/decode.c 2013-12-09 14:11:03.212344872 +0100
+++ libtar-1.2.16/lib/decode.c 2013-12-09 14:49:19.865470471 +0100
@@ -21,24 +21,54 @@
# include string.h
#endif
+char *
+safer_name_suffix (char const *file_name)
+{
+ char const *p, *t;
+ p = t = file_name;
+ while (*p)
+ {
+ if (p[0] == '.' p[0] == p[1] p[2] == '/')
+ {
+ p += 3;
+ t = p;
+ }
+ /* advance pointer past the next slash */
+ while (*p (p++)[0] != '/');
+ }
+
+ if (!*t)
+ {
+ t = .;
+ }
+
+ if (t != file_name)
+ {
+ /* TODO: warn somehow that the path was modified */
+ }
+ return (char*)t;
+}
/* determine full path name */
char *
th_get_pathname(TAR *t)
{
static char filename[MAXPATHLEN];
+ char *safer_name;
if (t-th_buf.gnu_longname)
- return t-th_buf.gnu_longname;
+ return safer_name_suffix(t-th_buf.gnu_longname);
+
+ safer_name = safer_name_suffix(t-th_buf.name);
if (t-th_buf.prefix[0] != '\0')
{
snprintf(filename, sizeof(filename), %.155s/%.100s,
- t-th_buf.prefix, t-th_buf.name);
+ t-th_buf.prefix, safer_name);
return filename;
}
- snprintf(filename, sizeof(filename), %.100s, t-th_buf.name);
+ snprintf(filename, sizeof(filename), %.100s, safer_name);
return filename;
}
Index: libtar-1.2.16/lib/extract.c
===
--- libtar-1.2.16.orig/lib/extract.c 2013-12-09 14:11:03.212344872 +0100
+++ libtar-1.2.16/lib/extract.c 2013-12-09 14:39:22.248955358 +0100
@@ -305,7 +305,7 @@ tar_extract_hardlink(TAR * t, char *real
linktgt = lnp[strlen(lnp) + 1];
}
else
- linktgt = th_get_linkname(t);
+ linktgt = safer_name_suffix(th_get_linkname(t));
#ifdef DEBUG
printf( == extracting: %s (link to %s)\n, filename, linktgt);
@@ -343,9 +343,9 @@ tar_extract_symlink(TAR *t, char *realna
#ifdef DEBUG
printf( == extracting: %s (symlink to %s)\n,
- filename, th_get_linkname(t));
+ filename, safer_name_suffix(th_get_linkname(t)));
#endif
- if (symlink(th_get_linkname(t), filename) == -1)
+ if (symlink(safer_name_suffix(th_get_linkname(t)), filename) == -1)
{
#ifdef DEBUG
perror(symlink());
Index: libtar-1.2.16/lib/internal.h
===
--- libtar-1.2.16.orig/lib/internal.h 2012-05-17 09:34:32.0 +0200
+++ libtar-1.2.16/lib/internal.h 2013-12-09 14:36:57.503866114 +0100
@@ -15,3 +15,4 @@
#include libtar.h
+char* safer_name_suffix(char const*);
