Hello Paul and others,

thank you for the patch and discussion. I think it is preferable to fix
the issue, even if it is currently documented.

I see though that since the beginning the discussion has focused on
preventing the issue during the extraction of the first tarball, cf. 
"Sounds like tar by default should refuse to create symlinks to outside
the working directory." as you wrote. Could the issue be prevented
during the extraction of the second tarball (when the symlinks are
actually being followed and unexpected files overwritten) instead? I am
not sure myself what would be better, but I see that bsdtar prevents the
issue during the second extraction and not the first, as mentioned in
one of the messages in the thread. (bsdtar prints
x ./target/sensitive: Cannot extract through symlink target/sensitive: Success
when "target" is a symlink to a directory containing "sensitive".) I am
actually starting to think that the bsdtar solution is better, see the
end, the last paragraph.

Regarding your patch, the documentation changes look ambiguous:

On Wed, Aug 13, 2025 at 06:55:44PM -0700, Paul Eggert wrote:
> +* By default, tar no longer extracts suspicious links as-is.
> +
> +These link to locations outside the extraction directory.
> +They can be hard or symbolic links.  Instead of extracting them,
> +tar now diagnoses the issue and creates safer replacement symbolic
> +links to targets that start with '_' instead of '/', and that have
> +'_.'  instead of '..'.

(...)

> diff --git a/doc/tar.texi b/doc/tar.texi
> index 2fe2c45c..b4a8dcae 100644
> --- a/doc/tar.texi
> +++ b/doc/tar.texi
> @@ -9527,10 +9537,17 @@ The interpretation of options in file lists is 
> disabled by
>  @cindex file names, absolute
>  
>  By default, @GNUTAR{} drops a leading @samp{/} on
> -input or output, and complains about file names containing a @file{..}
> -component.  There is an option that turns off this behavior:
> +input or output and complains about file names containing a
> +@file{..}@: component.  Also, when extracting it treats links
> +specially if they start with @samp{/} or contain @file{..}.
> +Two options turn off this behavior:
>  
>  @table @option
> +@opindex absolute-links
> +@item --absolute-links
> +When extracting, do not sanitize links to names that start with
> +@samp{/} or contain a @file{..}@: component.
> +
>  @opindex absolute-names
>  @item --absolute-names
>  @itemx -P
> @@ -9568,49 +9585,50 @@ for the information on how to handle this case.}.
>  Symbolic links containing @file{..} or leading @samp{/} can also cause
>  problems when extracting, so @command{tar} normally extracts them last;
>  it may create empty files as placeholders during extraction.
> +Also, @command{tar} ordinarily refuses to extract a hard or symbolic link to 
> a
> +file name that starts with @samp{/} or contains a @file{..}@:
> +if the link might point outside the extraction directory,
> +as these links are suspicious and can be used to attack your system.
> +Instead, @command{tar} diagnoses the issue and extracts a replacement
> +symbolic link that contains @samp{_} instead of leading @samp{/}
> +and @samp{_.}@: instead of @samp{..}.

The change to NEWS sounds like the change of behavior happens only for
symlinks that point outside the extraction directory, but the changes to
the manual mention all symlinks containing '..', without distinguishing
between symlinks that contain '..' but point inside the archive (should
be safe) and genuinely problematic symlinks that point outside. As you
wrote earlier, "It's OK for symlink contents to contain ".." so long as
it does not escape the current directory." So, which interpretation is
correct? If the latter (all symlinks containing '..' are affected), that
would be quite a regression, I think that such archives (containing '..'
symlinks) are commonplace.

I would like to try myself by building tar with your patch, but
unfortunately the patch does not apply. Have you produced the patch in
top of other changes that are not in the git repo? (I am getting this:
Applying: Don’t extract suspicious links as is
error: patch failed: src/common.h:757
error: src/common.h: patch does not apply
error: patch failed: src/extract.c:1530
error: src/extract.c: patch does not apply
error: patch failed: src/misc.c:914
error: src/misc.c: patch does not apply
Patch failed at 0001 Don’t extract suspicious links as is
)

I have not found the git objects mentioned in the "index ..." lines for
the three problematic files in the patch.

Actually, I suspect that even the sanitization of symlinks starting with
'/' will be problematic. With this change, creating a tarball of a
complete system and extracting it into a subdirectory (think whole
system backup and recovery, or creating a chroot from a working system,
if one wants to do something similar to debootstrap) will no longer work
with default options, since an OS install can contain plenty of such
symlinks (try ls -l /etc/alternatives for examples).

Best regards, Pavel


Reply via email to