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.
Powered by Openwall GNU/*/Linux - Powered by OpenVZ