Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory
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
> 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
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
[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
> 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
[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
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
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
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
-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-