bug#27986: 26.0.50; 'rename-file' can rename files without confirmation

[Paul Eggert]

> > and this will cause the victim to lose all the data in /home/victim/secret/bar 
> > even though the attacker is supposed to lack access to anything under 
> > /home/victim/secret.
>
> You mean, "lose data" because files in /tmp/foo will overwrite their
> namesakes in /home/victim/secret/bar?

Yes. It's not just losing data of course; the attacker can add files to the 
victim directory.

> If my understanding of the situation is correct, then such an attack
> will still be possible with the proposed change, if /tmp/bar exists,
> because Emacs will still issue two separate system calls, and the
> symlink can be created in-between, albeit at a price of deleting
> /tmp/bar first.  Right?

No, such an attack is not possible. Emacs will run rename_noreplace which will 
fail because /tmp/bar exists, then the attacker will remove /tmp/bar and replace 
it with a symlink, then Emacs will run rename ("/tmp/foo", "/tmp/bar") which 
will fail because a directory like /tmp is sticky and the victim cannot remove 
the attacker's symlink. The attack would fail in a different way if /tmp were 
not sticky: the victim would remove the attacker's symlink.

> > As icing on the cake, the current behavior of (rename-file A B) disagrees with 
> > its documentation when B is an existing directory.
>
> Well, 2/3 of it.  The 3rd instance, in the Emacs manual gets it right:

Unfortunately the manual does not get it right. The main part of that 3rd 
instance agrees with the proposed change, because it says the special case 
occurs if NEWNAME "is just a directory name" (on Unix, this means it ends in 
"/"). You are correct that the example is missing a "/", so I'll update the 
proposed patch to fix that.

That part of the documentation has some other confusion: a phrase "The same rule 
applies to all the remaining commands in this section" is clearly an editing 
error; it must be a revenant of when the section didn't talk about commands like 
insert-file. I just now installed the attached patch to fix that (this patch 
doesn't address the directory issue).

The bigger picture here is that this part of Emacs behavior is so poorly 
documented that it's unclear from the documentation what the intent was.

> How about eating the cake and having it, too?  We could refrain from
> testing whether B is a directory if either (1) B ends in a slash, or
> (2) rename_noreplace succeeds.

That doesn't close the security hole, I'm afraid. For example, the attacker can 
create a nonempty directory B, then rename_noreplace will fail, then Emacs will 
determine that B is a directory, then the attacker can replace B with a symlink 
to the victim directory, and then Emacs will overwrite the victim. I imagine 
other attacks are possible, this is just the first one off the top of my head.

> What I don't quite understand is what will happen under your proposal
> to the calls of the form (rename-file A B) where B names an existing
> directory and doesn't end in slash?  Will it fail, sometimes or
> always?

On POSIX systems rename-file will fail if B is a nonempty directory, and will 
succeed if B names an empty directory (this is all assuming B is not itself a 
directory name). Ideally MS-Windows would be compatible; if not, we'd have to 
document the incompatibility.

AFAIU, the 'rename' call will fail if B is a non-empty
> directory, but what if it's empty, and what does rename_noreplace do
> in these situations?  Your documentation patches don't cover this use
> case; I think we should.

Thanks, good point, I plan to update the proposed patch accordingly and to 
follow up soon.

0001-New-manual-section-Copying-and-Naming.patch
Description: Text Data