Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-09-29 Thread Petter Reinholdtsen
For the record, this issue is still flagged as unsolved upstream. :(
No activity in the bug tracker there since 2015 when Chris added the
last comment.
-- 
Happy hacking
Petter Reinholdtsen



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-06-29 Thread Chris Lamb
> busybox tar should do basically the same as GNU tar

Indeed. The implementation wasn't quite as straightforward or as clean as 
fixing the symlinks case, hence why my patch on upstream's bugtracker only 
addresses that part.


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-06-29 Thread Ben Hutchings
On Wed, 2016-06-29 at 10:39 +0200, Petter Reinholdtsen wrote:
> [Chris Lamb]
> > IIRC it was deemed to be low-priority from an LTS point of view so/and
> > I could not justify spending more time on it then. Happy to look again
> > if there is a more urgent requirement.
> 
> Right.  It is still unsolved in stable, testing, unstable and upstream,
> and the second oldest open CVE on my stable laptop (the oldest is in
> ruby, and pending a stable update), so I would like to see it fixed to
> reduce the number of known security problems on my machine. :)
> 
> Can not say much about the priority or urgency related to other issues,
> though. :)
> 
> I've poked upstream too, and hope some solution will materialise.

This was fixed in GNU tar some years ago, and I was able to implement a
similar fix in p7zip (thought that was simpler because p7zip doesn't
support hard links).

busybox tar should do basically the same as GNU tar, though without
copying code since they are unfortunately not licence-compatible.

Ben.

-- 

Ben Hutchings
Make three consecutive correct guesses and you will be considered an
expert.


signature.asc
Description: This is a digitally signed message part


Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-06-29 Thread Petter Reinholdtsen
[Chris Lamb]
> IIRC it was deemed to be low-priority from an LTS point of view so/and
> I could not justify spending more time on it then. Happy to look again
> if there is a more urgent requirement.

Right.  It is still unsolved in stable, testing, unstable and upstream,
and the second oldest open CVE on my stable laptop (the oldest is in
ruby, and pending a stable update), so I would like to see it fixed to
reduce the number of known security problems on my machine. :)

Can not say much about the priority or urgency related to other issues,
though. :)

I've poked upstream too, and hope some solution will materialise.

-- 
Happy hacking
Petter Reinholdtsen



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-06-29 Thread Chris Lamb
> Any idea why the resolution of this issue did not move any further?  I notice 
> from
> the upstream tracker that hardlinks might be a problem too.

Indeed, the hardlink part was blocking it.

IIRC it was deemed to be low-priority from an LTS point of view so/and I could 
not justify spending more time on it then. Happy to look again if there is a 
more urgent requirement.


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2016-06-29 Thread Petter Reinholdtsen
[Chris Lamb]
> (Small issues with patch; see upstream tracker)

Any idea why the resolution of this issue did not move any further?  I notice 
from
the upstream tracker that hardlinks might be a problem too.

-- 
Happy hacking
Petter Reinholdtsen



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2015-11-10 Thread Chris Lamb
tags 802702 - patch
forwarded 802702 https://bugs.busybox.net/8411
thanks

(Small issues with patch; see upstream tracker)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-



Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2015-11-05 Thread Chris Lamb
Updated patch attached.


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-
diff --git a/archival/libarchive/data_extract_all.c 
b/archival/libarchive/data_extract_all.c
index 45776dc..080718d 100644
--- a/archival/libarchive/data_extract_all.c
+++ b/archival/libarchive/data_extract_all.c
@@ -143,6 +143,40 @@ void FAST_FUNC data_extract_all(archive_handle_t 
*archive_handle)
case S_IFLNK:
/* Symlink */
 //TODO: what if file_header->link_target == NULL (say, corrupted tarball?)
+
+   /* To avoid a directory traversal attack via symlinks, for
+* certain link targets first write placeholder regular files
+* containing the intended target and mark to replace them
+* later.
+*
+* For example, consider a .tar created via:
+*
+*  $ tar cvf bug.tar anything.txt
+*  $ ln -s /tmp symlink
+*  $ tar --append -f bug.tar symlink
+*  $ rm symlink
+*  $ mkdir symlink
+*  $ tar --append -f bug.tar symlink/evil.py
+*
+* This will result in an archive that contains:
+*
+*  $ tar --list -f bug.tar
+*  anything.txt
+*  symlink [-> /tmp]
+*  symlink/evil.py
+*
+* Untarring bug.tar would otherwise place evil.py in '/tmp'.
+   */
+   if ((!strncmp(file_header->link_target, "/", 1))
+|| (strstr(file_header->link_target, "../"))) {
+   dst_fd = xopen(file_header->name, O_WRONLY | O_CREAT | 
O_EXCL);
+   xwrite(dst_fd, file_header->link_target,
+   strlen(file_header->link_target));
+   close(dst_fd);
+   llist_add_to_end(_handle->symlink_placeholders,
+   strdup(file_header->name));
+   break;
+   }
res = symlink(file_header->link_target, file_header->name);
if ((res == -1)
 && !(archive_handle->ah_flags & ARCHIVE_EXTRACT_QUIET)
diff --git a/archival/tar.c b/archival/tar.c
index bd61abd..37acbd6 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -22,24 +22,6 @@
  *
  * Licensed under GPLv2 or later, see file LICENSE in this source tree.
  */
-/* TODO: security with -C DESTDIR option can be enhanced.
- * Consider tar file created via:
- * $ tar cvf bug.tar anything.txt
- * $ ln -s /tmp symlink
- * $ tar --append -f bug.tar symlink
- * $ rm symlink
- * $ mkdir symlink
- * $ tar --append -f bug.tar symlink/evil.py
- *
- * This will result in an archive which contains:
- * $ tar --list -f bug.tar
- * anything.txt
- * symlink
- * symlink/evil.py
- *
- * Untarring it puts evil.py in '/tmp' even if the -C DESTDIR is given.
- * This doesn't feel right, and IIRC GNU tar doesn't do that.
- */
 
 //config:config TAR
 //config:  bool "tar"
@@ -309,6 +291,21 @@ static void chksum_and_xwrite(int fd, struct tar_header_t* 
hp)
xwrite(fd, hp, sizeof(*hp));
 }
 
+static void replace_symlink_placeholders(llist_t *list) {
+   char *target;
+   while (list) {
+   target = xmalloc_xopen_read_close(list->data, NULL);
+   if (unlink(list->data) == -1)
+   bb_error_msg_and_die("can't remove placeholder %s",
+   list->data);
+   if (symlink(target, list->data) == -1)
+   bb_error_msg_and_die("can't create link: %s -> %s",
+   list->data, target);
+   free(target);
+   list = list->link;
+   }
+}
+
 #if ENABLE_FEATURE_TAR_GNU_EXTENSIONS
 static void writeLongname(int fd, int type, const char *name, int dir)
 {
@@ -1205,6 +1202,8 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
while (get_header_tar(tar_handle) == EXIT_SUCCESS)
bb_got_signal = EXIT_SUCCESS; /* saw at least one header, good 
*/
 
+   replace_symlink_placeholders(tar_handle->symlink_placeholders);
+
/* Check that every file that should have been extracted was */
while (tar_handle->accept) {
if (!find_list_entry(tar_handle->reject, 
tar_handle->accept->data)
diff --git a/include/bb_archive.h b/include/bb_archive.h
index b82cfd8..bcaed7f 100644
--- a/include/bb_archive.h
+++ b/include/bb_archive.h
@@ -64,6 +64,9 @@ typedef struct archive_handle_t {
/* Currently processed file's header */
file_header_t *file_header;
 
+   /* List of symlink placeholders */
+   llist_t *symlink_placeholders;
+
/* Process the header component, e.g. tar -t */
void FAST_FUNC (*action_header)(const file_header_t *);
 
@@ -182,6 +185,7 @@ char 

Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2015-11-05 Thread Chris Lamb
tags 802702 + patch
forwarded 802702 https://bugs.busybox.net/show_bug.cgi?id=8411#c2
thanks

Patch attached & sent upstream.


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org / chris-lamb.co.uk
   `-
diff --git a/archival/libarchive/data_extract_all.c 
b/archival/libarchive/data_extract_all.c
index 45776dc..534c7fc 100644
--- a/archival/libarchive/data_extract_all.c
+++ b/archival/libarchive/data_extract_all.c
@@ -142,7 +142,40 @@ void FAST_FUNC data_extract_all(archive_handle_t 
*archive_handle)
break;
case S_IFLNK:
/* Symlink */
-//TODO: what if file_header->link_target == NULL (say, corrupted tarball?)
+
+   /* To avoid a directory traversal attack via symlinks, for
+* certain link targets first write placeholder regular files
+* containing the intended target and mark to replace them
+* later.
+*
+* For example, consider a .tar created via:
+*
+*  $ tar cvf bug.tar anything.txt
+*  $ ln -s /tmp symlink
+*  $ tar --append -f bug.tar symlink
+*  $ rm symlink
+*  $ mkdir symlink
+*  $ tar --append -f bug.tar symlink/evil.py
+*
+* This will result in an archive that contains:
+*
+*  $ tar --list -f bug.tar
+*  anything.txt
+*  symlink [-> /tmp]
+*  symlink/evil.py
+*
+* Untarring bug.tar would otherwise place evil.py in '/tmp'.
+   */
+   if ((!strncmp(file_header->link_target, "/", 1))
+|| (strstr(file_header->link_target, "../"))) {
+   dst_fd = xopen(file_header->name, O_WRONLY | O_CREAT | 
O_EXCL);
+   xwrite(dst_fd, file_header->link_target,
+   strlen(file_header->link_target));
+   close(dst_fd);
+   llist_add_to_end(_handle->symlink_placeholders,
+   strdup(file_header->name));
+   break;
+   }
res = symlink(file_header->link_target, file_header->name);
if ((res == -1)
 && !(archive_handle->ah_flags & ARCHIVE_EXTRACT_QUIET)
diff --git a/archival/tar.c b/archival/tar.c
index bd61abd..49fc93f 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -22,24 +22,6 @@
  *
  * Licensed under GPLv2 or later, see file LICENSE in this source tree.
  */
-/* TODO: security with -C DESTDIR option can be enhanced.
- * Consider tar file created via:
- * $ tar cvf bug.tar anything.txt
- * $ ln -s /tmp symlink
- * $ tar --append -f bug.tar symlink
- * $ rm symlink
- * $ mkdir symlink
- * $ tar --append -f bug.tar symlink/evil.py
- *
- * This will result in an archive which contains:
- * $ tar --list -f bug.tar
- * anything.txt
- * symlink
- * symlink/evil.py
- *
- * Untarring it puts evil.py in '/tmp' even if the -C DESTDIR is given.
- * This doesn't feel right, and IIRC GNU tar doesn't do that.
- */
 
 //config:config TAR
 //config:  bool "tar"
@@ -309,6 +291,20 @@ static void chksum_and_xwrite(int fd, struct tar_header_t* 
hp)
xwrite(fd, hp, sizeof(*hp));
 }
 
+static void replace_symlink_placeholders(llist_t *list) {
+   while (list) {
+   char *linkpath = list->data;
+   char *target = xmalloc_xopen_read_close(linkpath, NULL);
+   if (unlink(linkpath) == -1)
+   bb_error_msg_and_die("can't remove placeholder %s",
+   linkpath);
+   if (symlink(target, linkpath) == -1)
+   bb_error_msg_and_die("can't create link: %s -> %s",
+   linkpath, target);
+   list = list->link;
+   }
+}
+
 #if ENABLE_FEATURE_TAR_GNU_EXTENSIONS
 static void writeLongname(int fd, int type, const char *name, int dir)
 {
@@ -1205,6 +1201,8 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
while (get_header_tar(tar_handle) == EXIT_SUCCESS)
bb_got_signal = EXIT_SUCCESS; /* saw at least one header, good 
*/
 
+   replace_symlink_placeholders(tar_handle->symlink_placeholders);
+
/* Check that every file that should have been extracted was */
while (tar_handle->accept) {
if (!find_list_entry(tar_handle->reject, 
tar_handle->accept->data)
diff --git a/include/bb_archive.h b/include/bb_archive.h
index b82cfd8..bcaed7f 100644
--- a/include/bb_archive.h
+++ b/include/bb_archive.h
@@ -64,6 +64,9 @@ typedef struct archive_handle_t {
/* Currently processed file's header */
file_header_t *file_header;
 
+   /* List of symlink placeholders */
+   llist_t *symlink_placeholders;
+
/* Process the header 

Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

2015-10-22 Thread Henri Salo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Package: busybox
Version: 1:1.22.0-15
Severity: important
Tags: security, upstream

It was discovered that busybox's tar implementation will extract a symlink that
points outside of the current working directory and follow that symlink when
extracting other files. This allows for a directory traversal attack when
extracting untrusted tarballs.

This behavior is documented in the source code:

  http://git.busybox.net/busybox/tree/archival/tar.c#n25

More information:

  https://bugs.busybox.net/8411
  http://openwall.com/lists/oss-security/2015/10/21/4

- -- 
Henri Salo
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)

iQIcBAEBAgAGBQJWKSTaAAoJECet96ROqnV0sVgQANaMEz84St56AgwKRyiEh2U1
v8B8yaoIyGJA5H0mAbQV6lfVk48ueh0TFNFx4sanBTuR+tD++ibZSREnyG3xfzSf
U0aqqFGzQONAMMVbsIEzrd0hz+rwZKwchZbjMmjsiPLyexVTK+FDddC+5BsZBhEI
lcyFJYepiR1xXpI7uk2qv1j2+GRwDW7kDIipGWbyZIzBJMHHrsq/9VARteMb27BP
RoXWgr8UcAJ6Gc/wfQQQpLhv4EXZOH0BOL7lF2kLYzu754HiFfwcuU9bULwQ2VPH
8vKxFfiPDmcTO2cDjdzs5ofeRG0PyIURFSBpZ3u1OdanmANp9KM8+Ud5NHWCMtkM
JXtlZZ+4uySoJEtS6GlDw9h90bhXn8ufyRf4UNdiJsx4iB7EoX1AZZjdoCTvuurb
HpzAUTVFIXv6hi3V/3OY2iyZau51vAe7QsQ7moI0felYZvOWL17efLyySSJUB+lr
lXuaAla7fsvO/Y529YqBg+8jx29h4pi0HPCWS+cllf7Avo8fhor9Y0u8Ni4pn4yq
ISzrZZTJGwzKC15wkXUkB4DyzU9TxJBcAJ0CQlLKENFpU+yrYJ2rX1FPsdZJ86CL
r3eWbKWMQpUrmy6GGVOByROaGoo5/PBzcFq+66xQ1utVtHEp/4dtEau+iqU+BTat
54769j9aMKxKOdqs3hnl
=Ft7+
-END PGP SIGNATURE-