Subject: Re: tar ignores filenames that contain `..'
To: Todd Vierling <tv@pobox.com>
From: Greg A. Woods <woods@weird.com>
List: current-users
Date: 10/27/2002 17:53:29
[ On Saturday, October 26, 2002 at 11:17:32 (-0400), Todd Vierling wrote: ]
> Subject: Re: tar ignores filenames that contain `..'
>
> And one more round, after thinking about it last night.
>
> It occurred to me that, now that the assessment of the issue has changed
> from the content of symlinks to the act of *following* symlinks, that the
> protections mentioned in the proposal could be applied as default behavior,
> and all this can be distilled/simplified further.
>
> These corner cases shouldn't appear in normal archives, even for
> backup/restore operations, so they can be errors by default. Even
> pkg_add(8) won't need to use the --insecure option in this proposal, since
> binary packages won't contain such things.
>
> Revised proposal follows:
>
> 1. Create an "insecure mode" flag in pax, which will actually extract,
> rather than skip, the following types of entries (and suppress the
> related warnings?). (Flag already available: --insecure)
Using '-I' for 'pax' and '-U' for the 'tar' and 'cpio' front ends would
be nice -- there's no need to force use of GNU standards. :-)
> 2. For each entry being extracted, warn and skip file if any intervening
> path component is a symlink in the filesystem. (This catches both extant
> symlinks *and* those created by pax.)
Hmmm.... I _think_ that's a good idea. However if I'm not mistaken
implementing it in a truly secure (race-free) way in conjunction with
'tar -p' (or 'pax -p [oe]', or 'cpio -i' as root (without -R should it
be implemented)), at least when the directory was not created fresh from
the archive and even then unless setting of directory permissions is
held until the end, without additional kernel support will require some
extra work: open() each directory, fstat() to check if it's a
directory, and if so then fchdir() into it and repeat, then once in the
target's directory optionally unlink() the target file, open(O_EXCL) it,
fstat() it to make sure it's a regular file, and then write to it;
finally fchdir() back to the top and close the intervening directory
descriptors, optimizing this out for common directories as much as
possible.
The same checks should of course be done for 'pax' as 'pax', and 'pax'
as 'cpio' as well I think, though that'll probably happen by default so
long as nothing's done to prevent it given the way the implementation
works.
The enhancement from Openwall to prevent root from ever following
symlinks not owned by root might be easier and should be less overhead. :-)
> If the entry's full path is an extant symlink, however, don't warn; do
> standard unlink-and-create logic. Basically, don't do the lstat(2)
> check on the last path component. (This would have to be tested as to
> whether it DTRT for non-plain-files, e.g. directories and device nodes,
> that replace symlinks in the filesystem. The idea is that even a
> directory that exactly matches an extant symlink would simply replace the
> symlink with a directory safely.)
I thought this was already the way it was supposed to work, unless '-L'
was given.... Perhaps it's not quite fully implemented that way though
at the moment.
> 3. If a file is encountered in the archive which contains "../" in its
> pathname, warn and skip file.
I think that should read something more like this:
3. If a file is encountered in the archive with a pathname which
starts with or has as a second path component "../", or has any
string of N repeated components of '../' for every N preceding
normal directory components, warn and skip file.
i.e. a pathnames of of "foo/bar/../file" or "foo/bar/none/../../file",
shouldn't be a problem, should they? (assuming we've passed the tests
in #2) -- after all if those file already exist then the user is likely
unpacking an archive over exising files anyway and the entire purpose is
to overwrite existing files! (not that I have from-the-wild examples of
any archives containing such files....)
(that's just a first cut at the algorithm -- I didn't consider what
rules should be used to evaluate the count of "normal" path components
in the prefix, eg. if one such is "../" then it has to decrement the
count I guess)
--
Greg A. Woods
+1 416 218-0098; <g.a.woods@ieee.org>; <woods@robohack.ca>
Planix, Inc. <woods@planix.com>; VE3TCP; Secrets of the Weird <woods@weird.com>