diff --git a/NEWS b/NEWS index 866edcee..3cfe2b0d 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -GNU tar NEWS - User visible changes. 2025-11-09 +GNU tar NEWS - User visible changes. 2025-11-13 Please send GNU tar bug reports to version 1.35.90 (git) @@ -35,6 +35,9 @@ option. * Bug fixes +** When extracting, tar no longer follows symbolic links to targets + outside the working directory. + ** Fixed O(n^2) time complexity bug for large numbers of directories when extracting with --delay-directory-restore or reading incremental archives. diff --git a/doc/tar.texi b/doc/tar.texi index afc0c97e..e3c5474d 100644 --- a/doc/tar.texi +++ b/doc/tar.texi @@ -13128,26 +13128,31 @@ when you later extract from the archive you will get incorrect data. When @command{tar} extracts from an archive, by default it writes into files relative to the working directory. If the archive was generated by an untrusted user, that user therefore can write into any file -under the working directory. If the working directory contains a -symbolic link to another directory, the untrusted user can also write -into any file under the referenced directory. When extracting from an +under the working directory. When extracting from an untrusted archive, it is therefore good practice to create an empty -directory and run @command{tar} in that directory. +directory and run @command{tar} in that directory. You can use the +@option{--directory} (@option{-C}) option to specify the working +directory (@pxref{directory}). -When extracting from two or more untrusted archives, each one should -be extracted independently, into different empty directories. -Otherwise, the first archive could create a symbolic link into an area -outside the working directory, and the second one could follow the -link and overwrite data that is not under the working directory. For -example, when restoring from a series of incremental dumps, the -archives should have been created by a trusted process, as otherwise -the incremental restores might alter data outside the working -directory. +When extracting from an archive, @command{tar} rejects attempts to +modify files outside the working directory. +For example, if a symbolic link points outside the working directory, +@command{tar} refuses to follow the link, regardless of whether the +symbolic link existed before @command{tar} was run. +Therefore, when extracting from two or more untrusted archives, +each one can be extracted in turn, into the same initially-empty directory. +Even if an earlier archive creates a symbolic link that +points outside the working directory, +@command{tar} will reject any later attempts to follow that symbolic link. +However, this safety mechanism applies only to @command{tar} itself: +it does not apply to other programs you may run later, which will +ordinarily follow symbolic links even if they escape the working directory. If you use the @option{--absolute-names} (@option{-P}) option when extracting, @command{tar} respects any file names in the archive, even -file names that begin with @file{/} or contain @file{..}. As this -lets the archive overwrite any file in your system that you can write, +file names that begin with @file{/}, contain @file{..}, or that follow +a symbolic link to escape the extraction directory. As this lets the +archive overwrite any file in your system that you can write, the @option{--absolute-names} (@option{-P}) option should be used only for trusted archives. @@ -13217,7 +13222,7 @@ Protect archives at least as much as you protect any of the files being archived. @item -Extract from an untrusted archive only into an otherwise-empty +Extract from untrusted archives only into an otherwise-empty directory. This directory and its parent should be accessible only to trusted users. For example: @@ -13230,8 +13235,6 @@ $ @kbd{tar -xvf /archives/got-it-off-the-net.tar.gz} @end group @end example -As a corollary, do not do an incremental restore from an untrusted archive. - @item Do not let untrusted users access files extracted from untrusted archives without checking first for problems such as setuid programs. diff --git a/gnulib.modules b/gnulib.modules index dd643272..2640614d 100644 --- a/gnulib.modules +++ b/gnulib.modules @@ -84,6 +84,7 @@ mkfifoat modechange obstack openat +openat2 parse-datetime priv-set progname diff --git a/src/common.h b/src/common.h index 92032d22..4502d953 100644 --- a/src/common.h +++ b/src/common.h @@ -376,7 +376,7 @@ struct name /* Flags for reading, searching, and fstatatting files. */ extern int open_read_flags; -extern int open_searchdir_flags; +extern struct open_how open_searchdir_how; extern int fstatat_flags; extern int seek_option; diff --git a/src/create.c b/src/create.c index 5cecdcea..99a8aa34 100644 --- a/src/create.c +++ b/src/create.c @@ -1344,7 +1344,7 @@ create_archive (void) struct fdbase f = fdbase (p->name); int fd = (f.fd == BADFD ? -1 : openat (f.fd, f.base, - open_searchdir_flags)); + open_searchdir_how.flags)); if (fd < 0) { file_removed_diag (p->name, !p->parent, @@ -1569,7 +1569,7 @@ restore_parent_fd (struct tar_stat_info const *st) struct tar_stat_info *parent = st->parent; if (parent && ! parent->fd) { - int parentfd = openat (st->fd, "..", open_searchdir_flags); + int parentfd = openat (st->fd, "..", open_searchdir_how.flags); struct stat parentstat; if (parentfd < 0) @@ -1585,7 +1585,7 @@ restore_parent_fd (struct tar_stat_info const *st) { struct fdbase f = fdbase (parent->orig_file_name); int origfd = (f.fd == BADFD ? -1 - : openat (f.fd, f.base, open_searchdir_flags)); + : openat (f.fd, f.base, open_searchdir_how.flags)); if (0 <= origfd) { if (fstat (parentfd, &parentstat) < 0 diff --git a/src/misc.c b/src/misc.c index 59ff2851..d1b54ba9 100644 --- a/src/misc.c +++ b/src/misc.c @@ -1070,7 +1070,7 @@ chdir_do (idx_t i) if (! IS_ABSOLUTE_FILE_NAME (curr->name)) chdir_do (i - 1); fd = openat (chdir_fd, curr->name, - open_searchdir_flags & ~ O_NOFOLLOW); + open_searchdir_how.flags & ~O_NOFOLLOW); if (fd < 0) open_fatal (curr->name); @@ -1173,6 +1173,16 @@ fdbase_clear (void) } } +/* Starting from the directory FD, open a subdirectory SUBDIR for search. + If extracting or diffing and --absolute-names (-P) is not in effect, + do not let the subdirectory escape FD, i.e., the subdirectory must + be at or under FD in the directory hierarchy. */ +static int +open_subdir (int fd, char const *subdir) +{ + return openat2 (fd, subdir, &open_searchdir_how, sizeof open_searchdir_how); +} + /* Return an fd open to FILE_NAME's parent directory, along with the base name of FILE_NAME. Use the alternate cache if ALTERNATE, the main cache otherwise. @@ -1224,7 +1234,7 @@ fdbase_opendir (char const *file_name, bool alternate) { /* The new directory is a subdirectory of the old, so open relative to FD rather than to chdir_fd. */ - int subfd = openat (fd, &subdir[c->subdirlen], open_searchdir_flags); + int subfd = open_subdir (fd, &subdir[c->subdirlen]); if (subfd < 0) { /* Keep the old directory cached and report open failure, @@ -1251,7 +1261,7 @@ fdbase_opendir (char const *file_name, bool alternate) and add new info if the new directory can be opened. */ if (0 < c->subdirlen) close (fd); - fd = openat (chdir_fd, c->subdir, open_searchdir_flags); + fd = open_subdir (chdir_fd, c->subdir); if (fd < 0) { if (BADFD != -1 && fd < 0) diff --git a/src/tar.c b/src/tar.c index bebb1aaf..b7772981 100644 --- a/src/tar.c +++ b/src/tar.c @@ -111,7 +111,7 @@ idx_t archive_names; const char **archive_name_cursor; char const *index_file_name; int open_read_flags; -int open_searchdir_flags; +struct open_how open_searchdir_how; int fstatat_flags; int seek_option; bool unquote_option; @@ -2709,8 +2709,12 @@ decode_options (int argc, char **argv) #else int search_flags = O_SEARCH | noatime_flag; #endif - open_searchdir_flags = (search_flags | O_BINARY | O_CLOEXEC | O_DIRECTORY - | nofollow_flag); + open_searchdir_how.flags = (search_flags | nofollow_flag + | O_BINARY | O_CLOEXEC | O_DIRECTORY); + if (!absolute_names_option + && (subcommand_option == EXTRACT_SUBCOMMAND + || subcommand_option == DIFF_SUBCOMMAND)) + open_searchdir_how.resolve = RESOLVE_BENEATH; } fstatat_flags = dereference_option ? 0 : AT_SYMLINK_NOFOLLOW; diff --git a/tests/Makefile.am b/tests/Makefile.am index ff27c690..1677a567 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -140,6 +140,7 @@ TESTSUITE_AT = \ extrac28.at\ extrac29.at\ extrac30.at\ + extrac31.at\ filerem01.at\ filerem02.at\ grow.at\ diff --git a/tests/extrac31.at b/tests/extrac31.at new file mode 100644 index 00000000..5cc68a47 --- /dev/null +++ b/tests/extrac31.at @@ -0,0 +1,55 @@ +# Test suite for GNU tar. -*- Autotest -*- +# Copyright 2025 Free Software Foundation, Inc. +# +# This file is part of GNU tar. +# +# GNU tar is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# GNU tar is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +AT_SETUP([extracting untrusted incremental]) +AT_KEYWORDS([extract extrac31 --absolute-names]) + + +AT_TAR_CHECK([ + +# Extraction should not escape the extraction directory +# even when extracting multiple times to the same directory. +(umask 022 && mkdir -p dira/sub dirb/sym dirb/sub/sym ext victimdir victimexp) +ln -s .. dira/sub/dotdot +ln -s ../sub dira/sub/dot +ln -s dotdot/sub dira/sub/anotherdot +ln -s ../victimdir dira/sym +ln -s dotdot/../victimdir dira/sub/sym +echo b1 >dirb/sym/file1 +echo b2 >dirb/sub/sym/file2 +echo v >victimdir/expected +echo v >victimdir/file1 +echo v >victimdir/file2 +cp victimdir/* victimexp +tar -cf a.tar -C dira sub sym +tar -cf b.tar -C dirb sym/file1 sub/sym/file2 +tar -xf a.tar -C ext +echo status1=$? +tar -xf b.tar -C ext +echo status2=$? +diff victimdir victimexp +], +[], +[status1=0 +status2=2 +], +[tar: sym/file1: Cannot open: Invalid cross-device link +tar: sub/sym/file2: Cannot open: Invalid cross-device link +tar: Exiting with failure status due to previous errors +]) +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index e7e54f1e..c4912242 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -357,6 +357,7 @@ m4_include([extrac27.at]) m4_include([extrac28.at]) m4_include([extrac29.at]) m4_include([extrac30.at]) +m4_include([extrac31.at]) m4_include([backup01.at])