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

classic Classic list List threaded Threaded
39 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Philipp Stephani

Run the following in *scratch*:

(make-directory "/tmp/emacs")
nil
(write-region "" nil "/tmp/emacs/ß")
nil
(write-region "" nil "/tmp/emacs/ẞ")
nil
(directory-files "/tmp/emacs")
("." ".." "ß" "ẞ")
(rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
nil

Note how `rename-file' has silently overwritten `ß'.  This is because on
macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
equal.  Probably the test for case-insensitive file names should be
removed altogether (it can't work correctly and introduces a filesystem
race), and `rename-file' should use link(2) + unlink(2) if renameat2
isn't available.


In GNU Emacs 26.0.50 (build 77, x86_64-apple-darwin16.7.0, NS appkit-1504.83 Version 10.12.6 (Build 16G29))
 of 2017-08-06 built on p
Repository revision: b1b99edd3ee587a5154106d4d16547eac4916c55
Windowing system distributor 'Apple', version 10.3.1504
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --with-modules --without-xml2 --without-pop --with-mailutils
 --enable-gcc-warnings=yes MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo
 'CFLAGS=-O3 -g0' LDFLAGS=-O3'

Configured features:
DBUS NOTIFY ACL GNUTLS ZLIB TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win
ns-win ucs-normalize mule-util term/common-win tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow
isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 204143 9065)
 (symbols 48 20146 1)
 (miscs 40 43 181)
 (strings 32 29176 1731)
 (string-bytes 1 784929)
 (vectors 16 35010)
 (vector-slots 8 710995 9656)
 (floats 8 48 68)
 (intervals 56 205 0)
 (buffers 992 11))



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
> From: Philipp <[hidden email]>
> Date: Sun, 06 Aug 2017 17:40:18 +0200
>
> (rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
> nil
>
> Note how `rename-file' has silently overwritten `ß'.  This is because on
> macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
> equal.  Probably the test for case-insensitive file names should be
> removed altogether

Which one? there are two of them.

> (it can't work correctly and introduces a filesystem race)

It cannot work correctly _because_ of a possible race or because of
some other reasons?  If the latter, please elaborate.  If the former,
then at least on MS-Windows we have a race anyway, because the
underlying system APIs are not atomic.  So at least on MS-Windows the
feature should stay, as it introduces no new issues and supports a
valid and quite important use case (the Windows shell supports it as
well, so we really cannot do less).

> and `rename-file' should use link(2) + unlink(2) if renameat2
> isn't available.

'link' and 'unlink' accept strings as arguments, not integer numbers
such as 2.

More to the point, how can this strategy work on a case-insensitive
filesystem?  What am I missing?



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
In reply to this post by Philipp Stephani
>> Probably the test for case-insensitive file names should be
>> removed altogether
>
> Which one? there are two of them.

There should be just one such test, since it's testing the same thing and
there's no point to doing the same test twice. And I notice the code has some
other duplicate system calls. I installed the attached patch, which prunes away
some of these duplicates. With this patch, (rename-file "a" "b/") now takes just
one system call on recent GNU/Linux, whereas it used to take four (and was not
atomic).

However, even with this patch there are races on GNU/Linux which can lead to
potential security problems. Perhaps we can't fix these races on MS-Windows but
we should be able to fix them on a GNUish host. However, we will need to change
the semantics of rename-file etc. slightly, since no single system call supports
the cp-like target rewriting of these functions. I have a fix in mind to do that
in a hopefully compatible-enough way, which I'll try to propose soon. I'll keep
case-insensitive file systems in mind when I do that.

This reminds me of a similar problem in GNU coreutils which I fixed a dozen
years ago by adding the -T option to mv, ln, etc.

0001-Improve-performance-for-rename-file-etc.patch (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
Paul Eggert wrote:
> there are races on GNU/Linux which can lead to potential security problems.
> Perhaps we can't fix these races on MS-Windows but we should be able to fix them
> on a GNUish host. However, we will need to change the semantics of rename-file
> etc. slightly, since no single system call supports the cp-like target rewriting
> of these functions. I have a fix in mind to do that in a hopefully
> compatible-enough way, which I'll try to propose soon. I'll keep
> case-insensitive file systems in mind when I do that.

Attached is a proposed patch to fix this security problem. If I understand
things correctly, the fix should work on MS-Windows and on case-insensitive file
systems. Since this patch entails an incompatible change to the (undocumented)
behavior of (rename-file A B) when B is a directory but is not a directory name,
I'll mention the proposed change on emacs-devel.

0001-Fix-race-with-rename-file-etc.-with-dir-NEWNAME.patch (11K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
In reply to this post by Paul Eggert
Getting back to Philipp's original bug report, Apple documentation says macOS
has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
except it can be told to fail if the destination already exists). Attached is a
proposed patch to use this facility, which means that the case-insensitivity
test would no longer need to be done in macOS. If there's some way to implement
renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
there too.

I don't have easy access to macOS so I have not installed this patch. It'd be
nice, Philipp, if you could try it out.

This patch is independent of the destination-directory patch that I recently
proposed in:

https://bugs.gnu.org/27986#14

0001-Improve-rename-file-behavior-on-macOS.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Ken Brown-6
On 8/13/2017 7:48 PM, Paul Eggert wrote:
> Getting back to Philipp's original bug report, Apple documentation says
> macOS has a facility like the Linux renameat2 system call (i.e., it's
> like 'renameat' except it can be told to fail if the destination already
> exists). Attached is a proposed patch to use this facility, which means
> that the case-insensitivity test would no longer need to be done in
> macOS. If there's some way to implement renameat_noreplace on MS-Windows
> we could get rid of the case-insensitivity test there too.

I think we still need the case-insensitivity test on Cygwin as well as
on MS-Windows.

Ken




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
> From: Ken Brown <[hidden email]>
> Date: Mon, 14 Aug 2017 09:44:54 -0400
> Cc: [hidden email]
>
> I think we still need the case-insensitivity test on Cygwin as well as
> on MS-Windows.

Yes.  But I think there are more general issues with this, which I
will describe separately.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
In reply to this post by Paul Eggert
> From: Paul Eggert <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>, [hidden email]
> Date: Sun, 13 Aug 2017 16:48:59 -0700
>
> Getting back to Philipp's original bug report, Apple documentation says macOS
> has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
> except it can be told to fail if the destination already exists). Attached is a
> proposed patch to use this facility, which means that the case-insensitivity
> test would no longer need to be done in macOS. If there's some way to implement
> renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
> there too.

There's nothing easier than implementing renameat_noreplace on
MS-Windows, since the underlying system call does that by default, and
it's emulating the Posix behavior that requires complications.  In
fact, we already have this implementation: see sys_rename_replace (in
w32.c) which needs to be called with its last argument FALSE (modulo
the "at" part, which is easily handled).

(Ken, what about Cygwin?)

So I think we may be able to remove the case-sensitivity check right
now.  And in any case, that check should be in
barf_or_query_if_file_exists anyway, because renameat_noreplace on
case-insensitive filesystems already supports renaming to a different
letter-case.  The reason we had this check before is because we didn't
employ renameat_noreplace, but called 'rename' right away, and that
needed a question before overwriting the target.  But now
renameat_noreplace will silently succeed to rename when the user wants
to only change the letter-case, so no such check is needed.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
In reply to this post by Paul Eggert
> From: Paul Eggert <[hidden email]>
> Cc: Philipp <[hidden email]>, [hidden email]
> Date: Sun, 13 Aug 2017 15:42:05 -0700
>
> Paul Eggert wrote:
> > there are races on GNU/Linux which can lead to potential security problems.
> > Perhaps we can't fix these races on MS-Windows but we should be able to fix them
> > on a GNUish host.

For the record: 'rename' is atomic on Windows when the target doesn't
exist.  It's the case when the target exists that cannot be guaranteed
to be handled atomically, AFAIU, because deleting the old target and
renaming are not necessarily an atomic operation on Windows.  I don't
know if this is an issue, since the current code in rename-file uses 2
separate system calls in this case anyway, even on Posix platforms.

> Attached is a proposed patch to fix this security problem. If I understand
> things correctly, the fix should work on MS-Windows and on case-insensitive file
> systems. Since this patch entails an incompatible change to the (undocumented)
> behavior of (rename-file A B) when B is a directory but is not a directory name,
> I'll mention the proposed change on emacs-devel.

I'm uneasy, to say the least, to change the semantic of such a veteran
behavior.  Could you please take a step back and elaborate on the
races and the security problems related to this, and why the change in
the semantics you propose is the solution?  I'd like to understand the
problem better before we decide on the solution.

Thanks.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
In reply to this post by Eli Zaretskii
> Date: Mon, 14 Aug 2017 18:34:30 +0300
> From: Eli Zaretskii <[hidden email]>
> Cc: [hidden email], [hidden email]
>
> There's nothing easier than implementing renameat_noreplace on
> MS-Windows, since the underlying system call does that by default, and
> it's emulating the Posix behavior that requires complications.  In
> fact, we already have this implementation: see sys_rename_replace (in
> w32.c) which needs to be called with its last argument FALSE (modulo
> the "at" part, which is easily handled).

Which I just did.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Philipp Stephani
In reply to this post by Paul Eggert


Paul Eggert <[hidden email]> schrieb am Mo., 14. Aug. 2017 um 01:49 Uhr:
Getting back to Philipp's original bug report, Apple documentation says macOS
has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
except it can be told to fail if the destination already exists). Attached is a
proposed patch to use this facility, which means that the case-insensitivity
test would no longer need to be done in macOS. If there's some way to implement
renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
there too.

I don't have easy access to macOS so I have not installed this patch. It'd be
nice, Philipp, if you could try it out.


Thanks, the patch fixes the problem. However, it's still not 100% correct: now casing changes such as from "A" to "a" where macOS treats the file names as equivalent trigger the "file already exists" signal as well. I don't think that can be fixed, though; there's no way to special-case casing changes while keeping atomicity intact. So I'd rather have Emacs react conservatively and skip the casing check entirely.
Note that the manpage says:

RENAME_EXCL   On file systems that support it (see getattrlist(2) VOL_CAP_INT_RENAME_EXCL), it will cause EEXIST to be returned if the destination already exists. 

I interpret this such that if the filesystem doesn't support RENAME_EXCL the rename will succeed even if the destination exists.

Since we probably won't be able to solve all issues across operating systems and filesystems, probably we should have at least a warning in the documentation that rename-file attempts to be race-free and atomic, but only on a best-effort basis.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Philipp Stephani
In reply to this post by Eli Zaretskii


Eli Zaretskii <[hidden email]> schrieb am Mo., 14. Aug. 2017 um 17:34 Uhr:
> From: Paul Eggert <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>, [hidden email]
> Date: Sun, 13 Aug 2017 16:48:59 -0700
>
> Getting back to Philipp's original bug report, Apple documentation says macOS
> has a facility like the Linux renameat2 system call (i.e., it's like 'renameat'
> except it can be told to fail if the destination already exists). Attached is a
> proposed patch to use this facility, which means that the case-insensitivity
> test would no longer need to be done in macOS. If there's some way to implement
> renameat_noreplace on MS-Windows we could get rid of the case-insensitivity test
> there too.

There's nothing easier than implementing renameat_noreplace on
MS-Windows, since the underlying system call does that by default, and
it's emulating the Posix behavior that requires complications.

You might be able to eliminate these complications by calling MoveFileExW with MOVEFILE_REPLACE_EXISTING.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
> From: Philipp Stephani <[hidden email]>
> Date: Mon, 14 Aug 2017 16:58:04 +0000
> Cc: [hidden email]
>
>  There's nothing easier than implementing renameat_noreplace on
>  MS-Windows, since the underlying system call does that by default, and
>  it's emulating the Posix behavior that requires complications.
>
> You might be able to eliminate these complications by calling MoveFileExW with
> MOVEFILE_REPLACE_EXISTING.

Thanks, but I see no need, because using that function doesn't solve
any problems we have (it isn't guaranteed to be atomic if the
destination exists), and adds new complexity (because it won't move
files across volumes, and because it didn't exist before XP).  The
"complications" I mentioned above are already implemented and well
tested, so I see no point in eliminating them.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Philipp Stephani
In reply to this post by Eli Zaretskii


Eli Zaretskii <[hidden email]> schrieb am So., 6. Aug. 2017 um 19:05 Uhr:
> From: Philipp <[hidden email]>
> Date: Sun, 06 Aug 2017 17:40:18 +0200
>
> (rename-file "/tmp/emacs/ẞ" "/tmp/emacs/ß")
> nil
>
> Note how `rename-file' has silently overwritten `ß'.  This is because on
> macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
> equal.  Probably the test for case-insensitive file names should be
> removed altogether

Which one? there are two of them.

I guess all of them where correctness would depend on the outcome.
 

> (it can't work correctly and introduces a filesystem race)

It cannot work correctly _because_ of a possible race or because of
some other reasons?  If the latter, please elaborate. 

As this example shows, there are cases where two case-insensitive filenames are considered equivalent by Emacs, but different by the actual filesystem. This is unavoidable, because the definition of "case-insensitive" changes all the time, both in Emacs and in the filesystems. Generally it's impossible to detect whether two filenames would refer to the same file without actually creating the file. And even then the answer depends on how the file is created, see e.g. FILE_FLAG_POSIX_SEMANTICS. So Emacs can't compare filenames and make decisions based on the result upon which the correctness of a critical function depends. Comparing filenames can still be OK for best-effort attempts at giving the user better error messages or similar.
 
If the former,
then at least on MS-Windows we have a race anyway, because the
underlying system APIs are not atomic.

Wouldn't MoveFileExW with MOVE_FILE_REPLACE_EXISTING be atomic?

> and `rename-file' should use link(2) + unlink(2) if renameat2
> isn't available.

'link' and 'unlink' accept strings as arguments, not integer numbers
such as 2.

Yes, I mean the functions described in section 2 of the man page. link(2) is a common markup for this.
 

More to the point, how can this strategy work on a case-insensitive
filesystem?  What am I missing?

IIUC link(2) + unlink(2) would, if successful, guarantee enough atomicity in the sense that the old file is now guaranteed to be the new file, and the call is guaranteed to fail if the new file already exists. I don't think anything can help with the case-changing problem; I think we just have to live with an occasional false positive signal in this case. 
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
> From: Philipp Stephani <[hidden email]>
> Date: Mon, 14 Aug 2017 17:09:35 +0000
> Cc: [hidden email]
>
>  > Note how `rename-file' has silently overwritten `ß'. This is because on
>  > macOS, `ß' and `ẞ' are different file names, but Emacs treats them as
>  > equal. Probably the test for case-insensitive file names should be
>  > removed altogether
>
>  Which one? there are two of them.
>
> I guess all of them where correctness would depend on the outcome.

But then we lose a feature which just a few releases ago was deemed
valuable enough to have.

> As this example shows, there are cases where two case-insensitive filenames are considered equivalent by
> Emacs, but different by the actual filesystem. This is unavoidable, because the definition of "case-insensitive"
> changes all the time, both in Emacs and in the filesystems. Generally it's impossible to detect whether two
> filenames would refer to the same file without actually creating the file.

Wouldn't trying to rename it actually tell you that, by failing with EEXIST?
(On Windows, it simply succeeds and changes the letter-case silently,
but I understand that is not what happens on macOS.)

>  If the former,
>  then at least on MS-Windows we have a race anyway, because the
>  underlying system APIs are not atomic.
>
> Wouldn't MoveFileExW with MOVE_FILE_REPLACE_EXISTING be atomic?

No, it isn't guaranteed to be atomic.  No documentation says that,
certainly not any official documentation.  If I had access to the
sources, I could have looked there, but I don't.  Failing that, my
assumption is that it isn't atomic, because meta-data of more than one
file needs to be touched in this case.

> Yes, I mean the functions described in section 2 of the man page. link(2) is a common markup for this.

My point was that GNU coding standards frown on use of such markup.

>  More to the point, how can this strategy work on a case-insensitive
>  filesystem? What am I missing?
>
> IIUC link(2) + unlink(2) would, if successful, guarantee enough atomicity in the sense that the old file is now
> guaranteed to be the new file, and the call is guaranteed to fail if the new file already exists.

I actually think that if the old and the new name are equal but for
the letter-case, and the filesystem is case-insensitive, doing that
will delete the file, so you are left with nothing.

> I don't think anything can help with the case-changing problem; I
> think we just have to live with an occasional false positive signal
> in this case.

That'd be an unfortunate regression, IMO.  It isn't right for us to
back out changes we just introduced, which were considered important
when we did that.  We ought to find a solution that doesn't remove
this feature, not even on macOS.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
In reply to this post by Philipp Stephani
Philipp Stephani wrote:

> there's no way to special-case
> casing changes while keeping atomicity intact. So I'd rather have Emacs
> react conservatively and skip the casing check entirely.

Yes, that makes sense, at least for macOS.

> Note that the manpage says:
>
> RENAME_EXCL   On file systems that support it (see getattrlist(2)
> VOL_CAP_INT_RENAME_EXCL), it will cause EEXIST to be returned if the
> destination already exists.
>
> I interpret this such that if the filesystem doesn't support RENAME_EXCL
> the rename will succeed even if the destination exists.

I interpret it to mean that if the filesystem doesn't support RENAME_EXCL, the
rename will fail with errno == EINVAL or ENOSYS.  At least, that's how it works
under GNU/Linux. If it behaved the way you suggest, there'd be no good way to
emulate RENAME_EXCL on filesystems lacking it, which would surely not be Apple's
intent.

Please check this, though. I installed the patch on 'master' to help you do that.

Now that renameat_noreplace works on DOS_NT, would it make sense to apply the
attached further patch as well? If we can get renameat_noreplace to work on
Cygwin the we could simplify the fileio.c code even further.

> Since we probably won't be able to solve all issues across operating
> systems and filesystems, probably we should have at least a warning in the
> documentation that rename-file attempts to be race-free and atomic, but
> only on a best-effort basis.

True. I'd like to get the directory issue fixed before worrying about this,
though. (That way I don't have to document the security holes in the current
implementation. :-) I'll follow up separately about that.

0001-Improve-rename-file-behavior-on-DOS_NT.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
In reply to this post by Eli Zaretskii
Eli Zaretskii wrote:
> Could you please take a step back and elaborate on the
> races and the security problems related to this, and why the change in
> the semantics you propose is the solution?

Currently (rename-file A B) requires at least two system calls to work: one to
test whether B is a directory, and the other to actually do the rename. This
leads to race conditions if other actors change the file system between the two
calls.

For example, suppose a victim is about to execute (rename-file "/tmp/foo"
"/tmp/bar" t), and suppose an attacker wants to destroy the victim's file
/home/victim/secret/foo. The attacker can do (make-symbolic-link
"/home/victim/secret" "/tmp/bar"), 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. I doubt whether this is the only
such scenario; it's just the first one that popped into my mind.

As icing on the cake, the current behavior of (rename-file A B) disagrees with
its documentation when B is an existing directory.

There is no good solution to this problem. All solutions are bad, in that either
they are not 100% backward compatible with existing behavior, or they continue
to encourage insecure Elisp code. The proposed patch attempts to choose the
least bad way forward, by making the default behavior more secure, at a
relatively minor cost in compatibility. Most uses of rename-file etc. won't care
about the change, and the ones that do care are likely to have security problems
anyway.

The proposed solution improves security, because a common pattern in Lisp code
when creating a file BAR "atomically" is to create and write a temporary file
FOO and then execute (rename-file FOO BAR). Currently, this approach can be
attacked in the way described when BAR's parent directory is /tmp or any similar
directory. With the proposed patch, this approach cannot be hijacked in this
way, because BAR will be a file name and not a directory name. That is, the call
to rename-file will specify whether the destination-directory semantics are
desired, rather than relying on the state of the filesystem to specify it. This
is more secure because the state of the filesystem is partially under control of
attackers.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
In reply to this post by Paul Eggert
Paul Eggert wrote:
> I installed the patch on 'master' to help you do that.

After rereading the Apple manual I installed the attached further patch into
master, so please try updating to the latest master before testing.

0001-Improve-rename-file-port-to-macOS.patch (956 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Eli Zaretskii
In reply to this post by Paul Eggert
> Cc: Eli Zaretskii <[hidden email]>, [hidden email]
> From: Paul Eggert <[hidden email]>
> Date: Mon, 14 Aug 2017 16:03:13 -0700
>
> Philipp Stephani wrote:
>
> > there's no way to special-case
> > casing changes while keeping atomicity intact. So I'd rather have Emacs
> > react conservatively and skip the casing check entirely.
>
> Yes, that makes sense, at least for macOS.

As I wrote, we should not lose the feature that renaming a file on
macOS to a different letter-case works without nagging the user with
confirmations.  I don't think this runs afoul of security, since the
letter-case check doesn't break atomicity in any way.



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Paul Eggert
Eli Zaretskii wrote:
> we should not lose the feature that renaming a file on
> macOS to a different letter-case works without nagging the user with
> confirmations.

I was hoping that the current code does that as-is on macOS. Someone needs to
test that, though.



12
Loading...