Hi Sergey, commit e2b9cbd adds cpio_safer_name_suffix to stash_tar_linkname. Since read_in_tar_header calls that function for every type flag, SYMTYPE included, it now also rewrites symbolic link targets when reading tar and ustar archives with --no-absolute-filenames:
archive: abslink -> /usr/lib/libc.so.6 , dotdotlink -> ../sibling/data cpio -i --no-absolute-filenames tar/ustar: abslink -> usr/lib/libc.so.6 dotdotlink -> sibling/data cpio format: unchanged, both targets intact The ../ case is the worse one, because the link then resolves to a different existing path rather than to nothing. The warning that comes with it says "hard link targets" although the entry is a symlink. This is the behaviour you called "definitely a bug" in November 2019 and removed again in 376d663, with the note that 45b0ee2 "brought in more problems than solutions". The placeholder mechanism from 376d663 is still in place, so the protection it replaced 45b0ee2 with is unaffected. The patch gives stash_tar_linkname an argument telling it whether the target belongs to a symbolic link, and applies the guard only when it does not. Hard link and device entries keep being stripped, so the intent of e2b9cbd is preserved. I checked that with a hard link archive: the warning still appears and the link stays inside the extraction directory. The patch includes an autotest case. It fails before the change with -abslink=/usr/share/zoneinfo/UTC +abslink=usr/share/zoneinfo/UTC -dotdotlink=../bin/systemctl +dotdotlink=bin/systemctl and passes after it. The existing CVE-2015-1197 test does not catch this because it uses the cpio format, where the guard is not reached. Full suite here: 17 of 17 before, 18 of 18 after, no other change. If the test case needs copyright paperwork, please just take the src/tar.c part; the test is small enough to rewrite in whatever form suits you. One question while I am here: copyout.c:795 passes the result of readlink() through cpio_safer_name_suffix as well, so cpio -o --no-absolute-filenames stores rewritten symlink targets in the archive. That one is old, not from e2b9cbd, and for copy-out it may well be intended. Should it be changed too? Regards, Nexory https://github.com/Nexory The patch is inline below and attached as well, in case the inline copy does not survive the transport. --- src/tar.c | 21 ++++++++++++------- tests/Makefile.am | 3 ++- tests/symlink-tar-abs.at | 45 ++++++++++++++++++++++++++++++++++++++++ tests/testsuite.at | 3 +++ 4 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 tests/symlink-tar-abs.at diff --git a/src/tar.c b/src/tar.c index ff7af7c..9b26194 100644 --- a/src/tar.c +++ b/src/tar.c @@ -30,14 +30,19 @@ /* Stash the tar linkname in static storage. */ static char * -stash_tar_linkname (char *linkname) +stash_tar_linkname (char *linkname, bool symlink_target) { static char hold_tar_linkname[TARLINKNAMESIZE + 1]; strncpy (hold_tar_linkname, linkname, TARLINKNAMESIZE); hold_tar_linkname[TARLINKNAMESIZE] = '\0'; - cpio_safer_name_suffix (hold_tar_linkname, true, !no_abs_paths_flag, - false); + /* A hard link target names another member of the same archive, so it + is subject to --no-absolute-filenames. A symbolic link target is + stored verbatim and resolved only when the link is followed; + rewriting it would silently make the link point elsewhere. */ + if (!symlink_target) + cpio_safer_name_suffix (hold_tar_linkname, true, !no_abs_paths_flag, + false); return hold_tar_linkname; } @@ -363,7 +368,7 @@ read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des) be broken, and for device files with multiple links it puts the name of the link into linkname, but leaves typeflag as CHRTYPE, BLKTYPE, FIFOTYPE, etc. */ - file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname); + file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname, false); /* Does POSIX say that the filesize must be 0 for devices? We assume so, but HPUX's POSIX tar sets it to be 1 which causes @@ -375,27 +380,27 @@ read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des) break; case BLKTYPE: file_hdr->c_mode |= CP_IFBLK; - file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname); + file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname, false); file_hdr->c_filesize = 0; break; #ifdef CP_IFIFO case FIFOTYPE: file_hdr->c_mode |= CP_IFIFO; - file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname); + file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname, false); file_hdr->c_filesize = 0; break; #endif case SYMTYPE: #ifdef CP_IFLNK file_hdr->c_mode |= CP_IFLNK; - file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname); + file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname, true); file_hdr->c_filesize = 0; break; /* Else fall through. */ #endif case LNKTYPE: file_hdr->c_mode |= CP_IFREG; - file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname); + file_hdr->c_tar_linkname = stash_tar_linkname (tar_hdr->linkname, false); file_hdr->c_filesize = 0; break; diff --git a/tests/Makefile.am b/tests/Makefile.am index 83ddc23..e5096e5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -57,7 +57,8 @@ TESTSUITE_AT = \ CVE-2015-1197.at\ CVE-2019-14866.at\ linktime.at\ - linktime01.at + linktime01.at\ + symlink-tar-abs.at TESTSUITE = $(srcdir)/testsuite diff --git a/tests/symlink-tar-abs.at b/tests/symlink-tar-abs.at new file mode 100644 index 0000000..488418a --- /dev/null +++ b/tests/symlink-tar-abs.at @@ -0,0 +1,45 @@ +# Process this file with autom4te to create testsuite. -*- Autotest -*- +# Copyright (C) 2026 Free Software Foundation, Inc. +# +# This program 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, or (at your option) +# any later version. +# +# This program 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 <http://www.gnu.org/licenses/>. + +# --no-absolute-filenames must not rewrite symbolic link targets. A +# symlink target is stored verbatim and is resolved only when the link +# is followed, so stripping a leading `/' or `../' from it silently +# makes the link point somewhere else. Hard link targets, which name +# another member of the same archive, are still stripped. + +AT_SETUP([--no-absolute-filenames keeps symlink targets (tar format)]) + +AT_CHECK([ +mkdir src +cd src +ln -s /usr/share/zoneinfo/UTC abslink +ln -s ../bin/systemctl dotdotlink +printf 'abslink\ndotdotlink\n' | cpio -o -H ustar > ../test.tar 2>/dev/null +cd .. + +mkdir out +cd out +cpio -i --no-absolute-filenames < ../test.tar +echo "abslink=$(readlink abslink)" +echo "dotdotlink=$(readlink dotdotlink)" +], +[0], +[abslink=/usr/share/zoneinfo/UTC +dotdotlink=../bin/systemctl +], +[ignore]) + +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 24c27de..099c1e9 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -46,3 +46,6 @@ m4_include([CVE-2019-14866.at]) m4_include([linktime.at]) m4_include([linktime01.at]) + + +m4_include([symlink-tar-abs.at]) -- 2.53.0
0001-tar-keep-symbolic-link-targets-intact-under-no-absol.patch
Description: 0001-tar-keep-symbolic-link-targets-intact-under-no-absol.patch
