From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#27986: 26.0.50; 'rename-file' can rename files without confirmation Date: Tue, 15 Aug 2017 19:04:01 +0300 Message-ID: <83d17whl72.fsf@gnu.org> References: <61980dde-3d68-7200-e7f4-98f62e410060@cs.ucla.edu> <1002ee73-0ab5-409b-831f-0c283c322264@cs.ucla.edu> <83o9rignt6.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1502813123 3249 195.159.176.226 (15 Aug 2017 16:05:23 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 15 Aug 2017 16:05:23 +0000 (UTC) Cc: p.stephani2@gmail.com, 27986@debbugs.gnu.org To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Aug 15 18:05:17 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dheLB-0000It-Q3 for geb-bug-gnu-emacs@m.gmane.org; Tue, 15 Aug 2017 18:05:14 +0200 Original-Received: from localhost ([::1]:44135 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dheLI-0003VJ-4A for geb-bug-gnu-emacs@m.gmane.org; Tue, 15 Aug 2017 12:05:20 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dheL6-0003Qz-GH for bug-gnu-emacs@gnu.org; Tue, 15 Aug 2017 12:05:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dheL0-0006Ge-AN for bug-gnu-emacs@gnu.org; Tue, 15 Aug 2017 12:05:08 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:57938) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dheL0-0006GT-64 for bug-gnu-emacs@gnu.org; Tue, 15 Aug 2017 12:05:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dheKz-0007Vm-VD for bug-gnu-emacs@gnu.org; Tue, 15 Aug 2017 12:05:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 15 Aug 2017 16:05:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 27986 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: security Original-Received: via spool by 27986-submit@debbugs.gnu.org id=B27986.150281305628863 (code B ref 27986); Tue, 15 Aug 2017 16:05:01 +0000 Original-Received: (at 27986) by debbugs.gnu.org; 15 Aug 2017 16:04:16 +0000 Original-Received: from localhost ([127.0.0.1]:38384 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dheKG-0007VT-3a for submit@debbugs.gnu.org; Tue, 15 Aug 2017 12:04:16 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:35866) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dheKE-0007VN-R3 for 27986@debbugs.gnu.org; Tue, 15 Aug 2017 12:04:15 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dheK5-0005qS-C0 for 27986@debbugs.gnu.org; Tue, 15 Aug 2017 12:04:09 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:37462) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dheK5-0005qM-97; Tue, 15 Aug 2017 12:04:05 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:1888 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1dheK4-00074R-PT; Tue, 15 Aug 2017 12:04:05 -0400 In-reply-to: (message from Paul Eggert on Mon, 14 Aug 2017 16:31:38 -0700) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:135777 Archived-At: > Cc: p.stephani2@gmail.com, 27986@debbugs.gnu.org > From: Paul Eggert > Date: Mon, 14 Aug 2017 16:31:38 -0700 > > 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") You mean, the attacker creates this symlink between the time Emacs tests whether /tmp/bar is a directory and the time Emacs calls 'rename'? > 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? IOW, files in /home/victim/secret/bar which don't have identically-named counterparts in /tmp/foo will not be lost, right? (I'm not saying this is not a problem, I'm just trying to understand the meaning of "lose data" in this case.) 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? > 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: [...] If the argument NEW is just a directory name, the real new name is in that directory, with the same non-directory component as OLD. For example, ‘M-x rename-file ~/foo /tmp ’ renames ‘~/foo’ to ‘/tmp/foo’. Note that there's no trailing slash in "/tmp". > 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. 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. If B doesn't end in a slash and rename_noreplace fails, and the value of errno from rename_noreplace doesn't indicate B is a directory, then test if it's a directory and fall back on the old behavior. We could then change our code that calls rename-file to make B end in a slash, thus encouraging secure code and leaving the insecure behavior only as backward-compatibility feature. This will make the insecure code limited to situation which are already insecure due to a separate call to rename_noreplace. > 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. 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? 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.