When the creation of symlinks got delayed until after all other files
had been extracted, that didn't take into account that the archive may
contain hardlinks to those symlinks. The extraction of those hardlinks
did not get delayed so would fail.
This patch generalises the symlink_placeholders approach to a
link_placeholders which allows delaying the extraction of hardlinks too.
It changes the order in which delayed links are added to ensure that a
hardlink to an earlier symlink will create the symlink first.
It delays all hardlinks without regards to the target. It may be
desirable to instead tweak it to determine whether the link target is a
delayed symlink, but that is a cost/benefits tradeoff. As hardlinks are
relatively rare, I guess the benefit of such an optimisation would be
small, but I am not really qualified to make that determination.
Test case:
mkdir dir
>dir/a
ln -s ../dir/a dir/b
ln dir/b dir/c
mkdir new
tar cf - dir/* | tar -C new -xf -
busybox ad4e9613:
tar: can't create hardlink 'dir/c' to 'dir/b': No such file or directory
busybox ad4e9613 + patch:
No error output. The archive is successfully extracted.
The security hole that the delayed extraction of symlinks was meant to
prevent should not be re-introduced by the delayed creation of
hardlinks: the parent components of archive entries still get created
first, so if an archive contains a file "exploit", a symlink "parent ->
..", and a hardlink "parent/exploit -> exploit", the presence of that
hardlink causes "parent" to be immediately created as a regular
directory. The symlink creation "parent -> .." will therefore fail. This
could use careful review though.
Cheers,
Harald van Dijk
diff --git a/archival/cpio.c b/archival/cpio.c
index 308ec1b25..221667112 100644
--- a/archival/cpio.c
+++ b/archival/cpio.c
@@ -508,7 +508,7 @@ int cpio_main(int argc UNUSED_PARAM, char **argv)
while (get_header_cpio(archive_handle) == EXIT_SUCCESS)
continue;
- create_symlinks_from_list(archive_handle->symlink_placeholders);
+ create_links_from_list(archive_handle->link_placeholders);
if (archive_handle->cpio__blocks != (off_t)-1
&& !(opt & OPT_QUIET)
diff --git a/archival/libarchive/data_extract_all.c b/archival/libarchive/data_extract_all.c
index 8fa69ffaf..4c95db4a6 100644
--- a/archival/libarchive/data_extract_all.c
+++ b/archival/libarchive/data_extract_all.c
@@ -122,13 +122,10 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
/* Handle hard links separately */
if (hard_link) {
- res = link(hard_link, dst_name);
- if (res != 0) {
- /* shared message */
- bb_perror_msg("can't create %slink '%s' to '%s'",
- "hard", dst_name, hard_link
- );
- }
+ create_or_remember_link(&archive_handle->link_placeholders,
+ hard_link,
+ dst_name,
+ 1);
/* Hardlinks have no separate mode/ownership, skip chown/chmod */
goto ret;
}
@@ -195,9 +192,10 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
*
* Untarring bug.tar would otherwise place evil.py in '/tmp'.
*/
- create_or_remember_symlink(&archive_handle->symlink_placeholders,
+ create_or_remember_link(&archive_handle->link_placeholders,
file_header->link_target,
- dst_name);
+ dst_name,
+ 0);
break;
case S_IFSOCK:
case S_IFBLK:
diff --git a/archival/libarchive/unsafe_symlink_target.c b/archival/libarchive/unsafe_symlink_target.c
index 8dcafeaa1..f8dc8033d 100644
--- a/archival/libarchive/unsafe_symlink_target.c
+++ b/archival/libarchive/unsafe_symlink_target.c
@@ -5,13 +5,14 @@
#include "libbb.h"
#include "bb_archive.h"
-void FAST_FUNC create_or_remember_symlink(llist_t **symlink_placeholders,
+void FAST_FUNC create_or_remember_link(llist_t **link_placeholders,
const char *target,
- const char *linkname)
+ const char *linkname,
+ int hard_link)
{
- if (target[0] == '/' || strstr(target, "..")) {
- llist_add_to(symlink_placeholders,
- xasprintf("%s%c%s", linkname, '\0', target)
+ if (hard_link || target[0] == '/' || strstr(target, "..")) {
+ llist_add_to_end(link_placeholders,
+ xasprintf("%c%s%c%s", hard_link, linkname, '\0', target)
);
return;
}
@@ -23,17 +24,17 @@ void FAST_FUNC create_or_remember_symlink(llist_t **symlink_placeholders,
}
}
-void FAST_FUNC create_symlinks_from_list(llist_t *list)
+void FAST_FUNC create_links_from_list(llist_t *list)
{
while (list) {
char *target;
- target = list->data + strlen(list->data) + 1;
- if (symlink(target, list->data)) {
+ target = list->data + 1 + strlen(list->data + 1) + 1;
+ if ((*list->data ? link : symlink) (target, list->data + 1)) {
/* shared message */
bb_error_msg_and_die("can't create %slink '%s' to '%s'",
- "sym",
- list->data, target
+ *list->data ? "hard" : "sym",
+ list->data + 1, target
);
}
list = list->link;
diff --git a/archival/tar.c b/archival/tar.c
index 224cb47d1..7071c144d 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -1244,7 +1244,7 @@ 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 */
- create_symlinks_from_list(tar_handle->symlink_placeholders);
+ create_links_from_list(tar_handle->link_placeholders);
/* Check that every file that should have been extracted was */
while (tar_handle->accept) {
diff --git a/archival/unzip.c b/archival/unzip.c
index 0d00d8dc9..96b7ab69b 100644
--- a/archival/unzip.c
+++ b/archival/unzip.c
@@ -372,9 +372,10 @@ static void unzip_extract_symlink(llist_t **symlink_placeholders,
target[xstate.mem_output_size] = '\0';
#endif
}
- create_or_remember_symlink(symlink_placeholders,
+ create_or_remember_link(symlink_placeholders,
target,
- dst_fn);
+ dst_fn,
+ 0);
free(target);
}
#endif
@@ -990,7 +991,7 @@ int unzip_main(int argc, char **argv)
}
#if ENABLE_FEATURE_UNZIP_CDF
- create_symlinks_from_list(symlink_placeholders);
+ create_links_from_list(symlink_placeholders);
#endif
if ((opts & OPT_l) && quiet <= 1) {
diff --git a/include/bb_archive.h b/include/bb_archive.h
index a5c61e95b..4492ffe59 100644
--- a/include/bb_archive.h
+++ b/include/bb_archive.h
@@ -64,8 +64,8 @@ typedef struct archive_handle_t {
/* Currently processed file's header */
file_header_t *file_header;
- /* List of symlink placeholders */
- llist_t *symlink_placeholders;
+ /* List of link placeholders */
+ llist_t *link_placeholders;
/* Process the header component, e.g. tar -t */
void FAST_FUNC (*action_header)(const file_header_t *);
@@ -199,10 +199,11 @@ void seek_by_jump(int fd, off_t amount) FAST_FUNC;
void seek_by_read(int fd, off_t amount) FAST_FUNC;
const char *strip_unsafe_prefix(const char *str) FAST_FUNC;
-void create_or_remember_symlink(llist_t **symlink_placeholders,
+void create_or_remember_link(llist_t **link_placeholders,
const char *target,
- const char *linkname) FAST_FUNC;
-void create_symlinks_from_list(llist_t *list) FAST_FUNC;
+ const char *linkname,
+ int hard_link) FAST_FUNC;
+void create_links_from_list(llist_t *list) FAST_FUNC;
void data_align(archive_handle_t *archive_handle, unsigned boundary) FAST_FUNC;
const llist_t *find_list_entry(const llist_t *list, const char *filename) FAST_FUNC;
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
