[Bug binutils/26945] Unsafe chown+chmod in smart_rename, possibly elsewhere

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=26945

--- Comment #9 from Rich Felker <bugdal at aerifal dot cx> ---
> > 1. make_tempname with fd_return==NULL is *always* a bug,

> Well, I would argue that it is only a bug if the temporary file is going to
be manipulated by other system functions.

Are you going to create a temp file but never write anything to it? That seems
pointless. As soon as you open it for write, you have bug, and probably an
exploitable one. There is fundamentally no way of knowing it's the same temp
file you created anymore since the only reference you retained to it is a name,
which is ephemeral.

> > 2. smart_rename needs(*) *two* fds, not just one

> Actually it does this.  It is just that only one fd is passed in (the open 
> temporary file).  The function itself opens the destination file before it
attempts to perform any other operations on it.

It doesn't. The second file descriptor I'm talking about is the one to the
original file that was read from, that will be replaced by the rename. This is
the one you need to read old ownership/mode from with fstat. Using the name
here is not safe (wrt using the right data). As long as you use O_NOFOLLOW and
check that the link count is 1, I don't think it's *exploitably* wrong, but I
wouldn't want to bet on that. It's still conceptually wrong and should just be
done the right way.

-- 
You are receiving this mail because:
You are on the CC list for the bug.