[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 28 Nov 2017 14:19:58 +0100
From: Bram Moolenaar <Bram@...lenaar.net>
To: Solar Designer <solar@...nwall.com>
Cc: oss-security@...ts.openwall.com
Subject: Re: Security risk of server side text editing ...
Solar Designer wrote:
> On Mon, Nov 27, 2017 at 02:10:54PM -0500, Scott Court wrote:
> > Here's the summary you asked for. As far as I've been able to tell,
> > there are three vulnerabilities being discussed here:
>
> Thank you for trying to do the right thing, but I'm concerned that with
> the "multiple vulnerabilities in Vim" approach we can identify lots of
> them and "fix" lots of them yet not achieve a sensible goal - in fact,
> we already started down that path.
>
> > 1. CVE-2017-1000382
>
> Kurt assigned this one to:
>
> "Please use CVE-2017-1000382 for VIM version 8.0.1187 (and other versions
> most likely) ignores umask when creating a swap file
> (\"[ORIGINAL_FILENAME].swp\") resulting in files that may be world readable
> or otherwise accessible in ways not intended by the user running the vi
> binary."
I still haven't found any reason why umask would apply to intermediate
or temporary files. It is intended to be used for newly created files.
Setting the access of the swap file equal to the edited file is the
right thing to do.
> While ignoring of umask might be the CVE-worthy issue here, in practice
> most of the files Hanno found were probably from systems that had most
> distros' default non-root user umask of 022 or 002 anyway.
>
> So fixing this CVE as worded does little to address Hanno's findings.
>
> > This vulnerability was discovered by Hanno Bock. 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.
>
> This is a good summary of the actual issue that we should address, but
> the CVE is only partially related to it. Fixing the CVE (as described)
> means honoring the umask, but it would do little to reduce the number of
> vulnerable sites Hanno would find. Improving Vim to always use 0600
> might or might not be considered a fix for the CVE (maybe 0600 & ~umask
> would be, even if anything stricter than 0600 breaks the functionality),
> but it would do much more to address Hanno's findings (only leaving out
> the special case of the web server running as the same (pseudo-)user who
> edits the files, which I expect is less common than editing with umask
> 022 or 002).
>
> So let's focus on what actually matters rather than on what fits a CVE.
>
> > 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.
>
> I agree this is a separate issue from ignoring the umask, and is
> probably a CVE-worthy vulnerability as well (if the first CVE is in fact
> limited to the umask), but for practical purposes there's just one thing
> to change in response to both issues: make those files 0600.
>
> > 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.
>
> This not a vulnerability in Vim, and is not CVE-worthy. Most programs,
> text editors included, are unsafe to use on pathnames in untrusted
> directories, period. We might want to harden Vim to make it less unsafe
> when misused like that, but calling that a vulnerability and those
> changes a fix would encourage the misuses and further misunderstandings.
>
> If I understand correctly, the specific example posted by Roman relied
> on a file being replaced with a symlink, so this falls in the above
> category - misuse of Vim in an untrusted directory.
I have fixed this anyway, patch 8.0.1300.
> OTOH, as an exception, Vim can and should be made safe when editing a
> file with an untrusted owner if that file (the specific hard link to it
> corresponding to the pathname being edited) is located in a trusted
> directory (including all of its parent directories) - e.g., /var/run/foo
> where only foo itself is under a potential attacker's control. If Vim
> would sometimes copy the SUID/SGID bits from such file to another
> (temporary) file that does not yet have the user and group ownership
> also already copied to it (which would have already been racy as well),
> then that's a concern and is something that should be fixed - again, by
> simply setting those files' permissions to 0600 (no need for any fancy
> logic). I don't know, and am not too interested, whether such an issue
> currently exists (I think it does not, as I saw code masking the
> permissions to 0777), nor whether it's CVE-worthy (it might be if it
> exists). I care more about the trivial proper change we should make in
> response to all of these non-misuse issues at once, without needing to
> enumerate and categorize them.
>
> If upstream approaches this differently, distros should throw the fancy
> logic out and hard-code 0600.
>
> > I believe these are the three big ones
>
> Thanks, but I think enumerating and categorizing them is a distraction,
> except to the extent necessary for us to get on the same page and make
> the trivial change. ;-)
--
I started out with nothing, and I still have most of it.
-- Michael Davis -- "Tonight Show"
/// Bram Moolenaar -- Bram@...lenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
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.
