Re: Re: Security risk of server side text editing ...

[Scott Court <z5t1 () z5t1 com>]

Hi Kurt,

Here's the summary you asked for. As far as I've been able to tell,
there are three vulnerabilities being discussed here:


    1. CVE-2017-1000382

This vulnerability was discovered by Hanno Böck. When editing a text
file in Vim, a .swp file is created in the same directory (if you edit
"foo", the swap file will be ".foo.swp"). Hanno pointed out that this
could create a security vulnerability on PHP enabled webservers as follows:

If a user goes to edit a .php file in the public_html directory (say
"foo.php"), a swap file will be created in the public_html directory
called ".foo.php.swp". This then exposes the contents of the PHP script
foo.php to the world. All someone has to do is go to
"http://example.com/.foo.php.swp"; and he can view the .swp file which
contains the contents of the original foo.php file.

Hanno pointed out that this causes a problem with Wordpress sites if the
site administrator edits the wp-config.php file in Vim: he exposes all
of the database credentials. This is made worse if Vim crashes while he
is editing it as then the .wp-config.php.swp file sticks around. He
claims he has found 750 websites that are vulnerable to this.


    2. Vim .swp file group (Doesn't have a CVE ID)

This vulnerability was discovered by me. When Vim creates a .swp file,
the .swp file is created with the owner and group set to the editor and
editor's primary group respectively. The .swp file is the set to the
same permissions as the original file (i.e. chmod 640). This creates a
security vulnerability when the editor's primary group is not the same
as the original file's group.

For example, say the root user's primary group is "users", which every
user is a member of. If root goes to edit /etc/shadow, the
/etc/.shadow.swp file is created with permissions 640 and user:group set
to root:users. The original /etc/shadow file had user:group set to
root:shadow though; this now exposes the /etc/shadow file (which mind
you contains hashes of every user's password) to every user on the system.

Originally, I thought this was an extension of CVE-2017-1000382 so I
didn't bother trying to get a CVE ID for it; however, upon looking at it
for a second time, it seems that this is indeed a different
vulnerability. It is possible to patch this vulnerability without
patching CVE-2017-1000382.


    3. Vim.tiny race condition (Doesn't have a CVE ID as far as I know)

I'm not quite sure who discovered this vulnerability (I don't use or
follow vim.tiny); however, it has been discussed on here so I will
include my limited knowledge of it for completeness sake. This is a race
condition in which a world writable SUID binary is temporarily created.
This could (or course) theoretically allow an arbitrary user to write to
that binary and execute arbitrary code as root; however, there is debate
as to whether or not doing this is actually feasible.

---

I believe these are the three big ones; however, I may have missed
something. There has been a lot of discussion about this family of
vulnerabilities lately. There are definitely at least these three
though. I'm sure if I've missed anything everyone else on this mailing
list will be more than happy to let me know.

Sincerely,

Scott Court


On 11/22/2017 05:27 PM, Kurt Seifried wrote:
> Can you post a summary of the issues, it sounds like more than one CVE will be needed, thanks.
>
>
> -Kurt
>
>
>
>
>
> > On Nov 22, 2017, at 15:17, Solar Designer <solar () openwall com> wrote:
> >
> > > 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
> > <8.0.1300>

Attachment: signature.asc
Description: OpenPGP digital signature