Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171122221706.GA26704@openwall.com>
Date: Wed, 22 Nov 2017 23:17:06 +0100
From: Solar Designer <solar@...nwall.com>
To: Bram Moolenaar <Bram@...lenaar.net>
Cc: oss-security@...ts.openwall.com
Subject: Re: Security risk of server side text editing ...

On Fri, Nov 17, 2017 at 11:35:15AM +0100, Bram Moolenaar wrote:
> Please check out patch 8.0.1300.

Thanks.  Personally, I don't have much to add.  This continues to do
what I find are weird and wrong things, so any implementation issues are
secondary to that.  I suppose you have some rationale for preserving the
old behavior of propagating the edited file's permissions onto related
temporary files, but I'm unaware of good reasons for that.

If it's about users' collaboration, then I don't see a good reason for
other users in the group, even if they could access the original file
via group permissions, to also have access to recovery and backup files.

As to the patch itself, aside from it propagating the possibly unsafe
permissions on purpose (I mean unsafe such as in Hanno's original
example, but also applying to backup files), it's also risky in
temporarily setting umask to 0.  On some systems, this could mean libc
or the kernel creating files with unsafe permissions if anything goes
very wrong during this time - e.g., a coredump.  Checking st_ino is OK
as a hardening measure, but might not always be sufficient: inode number
reuse is possible if the original file could have been deleted.
I suppose st_dev is not checked because of the use of O_NOFOLLOW, but I
guess Vim can be built on systems without working O_NOFOLLOW as well?

In case anyone wants to review the patch for real, I've attached it to
this message, and here it is on GitHub (for expanding of the context):

https://github.com/vim/vim/commit/cd142e3369db8888163a511dbe9907bcd138829c

Alexander

View attachment "8.0.1300" of type "text/plain" (12152 bytes)

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.