sftp rename not atomic on freebsd/aix with sticky bit enabled

[Damien Miller]

On Fri, 1 Dec 2023, Jozef Riha wrote:

> Hello list,
> 
> we've recently came across an interesting problem on one of our AIX
> systems which we were unable to reproduce on Linux host but we
> discovered that FreeBSD (11) is showing the same behaviour.
> 
> Preparation:
>  - on AIX/FreeBSD system inside /tmp with the standard 1777
> permissions create an empty file
>     touch foofile
>     chmod 600 foofile
>     chown root:system foofile
> 
> To reproduce:
> - in WinSCP login as a regular (non-root) user, browse to /tmp
> - navigate to /tmp/foofile, bring up a context menu (right-click),
> select Rename (F2)
> - receive a pop-up "Permission denied", click Abort
> - refresh directory listing
> - notice a new barfile (hardlink to foofile)
> 
> Expectation:
> - hardlink should not be present (rename is supposed to be atomic
> hence either it is processed fully or not at all).
> 
> It all comes down to a different behaviour of ln (link) on different systems:
> # AIX/FreeBSD, regular user inside /tmp
> ln foofile barfile # rc=0; link is created
> 
> # Linux
> ln foofile barfile # rc=1 (error: Operation not permitted); link is not created
> 
> Due to the directory having sticky bit on, the operation of renaming
> for AIX/FreeBSD fails in the very last stage (unlink), leaving a file
> behind.
> 
> As a workaround posix-rename (which is not race-free) could be used
> but there seems to be a limited support among GUI clients (for WinSCP
> there is a ticket opened to add this feature:
> https://winscp.net/tracker/2231).
> 
> Is there a way to address this problem in OpenSSH code-base?

I doubt it - sftp-server already tries to clean up when a legacy rename
operation fails. See the logic starting here:

https://github.com/openssh/openssh-portable/blob/V_9_5_P1/sftp-server.c#L1285

If file permissions are preventing this then there isn't anything more it
can do AFAIK. The SSH2_FXP_RENAME operation was just badly specified in
the sftp protocol, which is why we added posix-rename.

-d