Bug#330350: [Pkg-shadow-devel] Bug#330350: passwd: Potential symlink attack problem in remove-shell?

[Alexander Gattin]

Hi!

On Wed, Sep 28, 2005 at 06:47:50AM +0200, Christian Perrier wrote:
> 
> > This doesn't look that bad to me.
> > Here, the temporary file is in /etc/. If somebody can create a symlink in
> > /etc/, she can probably also change /etc/shadow.
> 
> Yes, right.
> 
> However, don't you think we'd better use a non-predictable temporary
> file name ?

IMHO we will just get more PITA with this especially if
we are going to rewrite these utils in C (but you know, I
consider this unnecessary).

<offtopic>
What is more interesing, is a general "atomic fs ops"
concept here. The author wanted to use _atomic mv_
(`mv $otmpfile $tmpfile` and `mv $tmpfile $file`),
hence requirement for having files and tmpfiles on the
same FS.

But in general, UNIX is deficient of well-thought
atomicity concept.

Also there's no well-thought temporary or _anonymous_
files concept either... :-/
</offtopic>

IMHO for these utils (add|remove-shell) the best way
would be:
1. lock (lockf) /etc/shells
2. creating _anonymous_ tmpfile[s]
3. put processed data there
4. link _prepared_ _anonymous_ file in place of
   /etc/shells
5. and unlock

In Unices the 4 is impossible. You can create anonymous
file by unlinking, but this operation isn't reversible,
i.e. you can't link it back into visible FS. Thus
unlinked file will remain visible only through (and
as long as there is) an open FD.

Also, it's not possible to create anonymous (unlinked)
storage directly, i.e. in one step. /* in Unices you
first create file then unlink */

This is what I hate the most in Unices -- this stupid
handling of tmp data and all those ugly wrappers
(tmpfile, mktemp, mkstemp) built around because of
security reasons...

And, returning to the topic -- I'm strongly against
using randomized tmpfiles for add|remove-shell because
they will _clutter_ /etc if the scripts will
fail/abort/coredump/whatever. Under /var/tmp this
garbage is cleaned regularly but who will take care of
/etc???

/* BTW, anonymous files won't clutter anything ;)
 * except maybe for free-space-bitmap, which is
 * refreshed/updated IIRC regularly. */

P.S. IIRC there are truly anonymous files in VMS.

P.P.S How do I imagine anonymous files support in POSIX:
> int anonfd = open("/etc", O_CREAT | O_RDWR | O_ANON, ...);
> /* process data: */
> ...
> if (-1 == linkfd(anonfd, "/etc/shells")) {
>     switch (errno) {
>     /* see e.g. man 2 rename: */
>     case EACCES: ...; exit(xxx);
>     case EXDEV: ...; exit(yyy);
>     case EISDIR: ...; exit(zzz);
>     ...
>     }
> }
> close(fd);
> exit(0);

Here you can see that 1 new syscall and 1 new flag
for open() are sufficient...

-- 
WBR,
xrgtn