bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eli Zaretskii
Subject: bug#27986: 26.0.50; 'rename-file' can rename files without confirmation
Date: Tue, 15 Aug 2017 20:42:46 +0300

> Cc: p.stephani2@gmail.com, 27986@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 15 Aug 2017 10:24:03 -0700
> 
> > 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

How would they know to create B before Emacs issues any system call
that uses B?

And how is this case different from the case that Emacs calls
(rename-file A B) thinking B doesn't exist (e.g., because some prior
code tested that)?

Anyway, I firmly believe we should be backward compatible as a
fallback.  It is okay for the fallback to be insecure, as the current
code is also insecure.  But I don't think we should fail use cases
that previously were legitimate, for many years.  If my proposal is
not workable, we should come up with something that is.

> > 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.

I see no problems in being compatible in this sense.  But wiping out
the empty directory instead of moving the first argument into it is
an incompatible change, and we should avoid that.

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

Please say in the manual explicitly that what you call "directory
name" should end in a slash.  Yes, I know this is already described
elsewhere in the manual, but since this is important in this case, it
doesn't hurt repeating it.  It's especially important in the doc
string and in lispref manual, since there the slash must be explicit,
whereas in the interactive usage I believe RET might complete the
slash.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]