patch 9.1.1947: [security]: Windows: Vim may execute commands from current directory

Problem:  [security]: Windows: Vim may execute commands from current
          directory (Simon Zuckerbraun)
Solution: Set the $NoDefaultCurrentDirectoryInExePath before running
          external commands.

Github Advisory:
https://github.com/vim/vim/security/advisories/GHSA-g77q-xrww-p834

Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt
index 0b5215d7ef1605b8816e09d426afea9e4b0830b2..5f51af3ad5397c9a5be8ddbf9765941aed4a98c7 100644 (file)
--- a/runtime/doc/builtin.txt
+++ b/runtime/doc/builtin.txt
@@ -2711,13 +2711,15 @@ executable({expr})                                      *executable()*
                then the name is also tried without adding an extension.
                On MS-Windows it only checks if the file exists and is not a
                directory, not if it's really executable.
+
                On MS-Windows an executable in the same directory as the Vim
                executable is always found.  Since this directory is added to
                $PATH it should also work to execute it |win32-PATH|.
                                        *NoDefaultCurrentDirectoryInExePath*
                On MS-Windows an executable in Vim's current working directory
                is also normally found, but this can be disabled by setting
-               the $NoDefaultCurrentDirectoryInExePath environment variable.
+               the `$NoDefaultCurrentDirectoryInExePath` environment variable.
+               This is always done for |:!| commands, for security reasons.
 
                The result is a Number:
                        1       exists

diff --git a/src/os_win32.c b/src/os_win32.c
index 182d005aa879d525ae95ee5d55a54925283ce503..0a25d48506d92e74b8fce38be1f1e752e112058c 100644 (file)
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -5483,6 +5483,21 @@ mch_call_shell_terminal(
     return retval;
 }
 #endif
+/* Restore a previous environment variable value, or unset it if NULL.
+ * 'must_free' indicates whether 'old_value' was allocated.
+ */
+    static void
+restore_env_var(char_u *name, char_u *old_value, int must_free)
+{
+    if (old_value != NULL)
+    {
+       vim_setenv(name, old_value);
+       if (must_free)
+           vim_free(old_value);
+       return;
+    }
+    vim_unsetenv(name);
+}
 
 /*
  * Either execute a command by calling the shell or start a new shell
@@ -5495,6 +5510,8 @@ mch_call_shell(
     int                x = 0;
     int                tmode = cur_tmode;
     WCHAR      szShellTitle[512];
+    int                must_free;
+    char_u     *oldval;
 
 #ifdef FEAT_EVAL
     ch_log(NULL, "executing shell command: %s", cmd);
@@ -5519,6 +5536,11 @@ mch_call_shell(
            }
        }
     }
+    // do not execute anything from the current directory by setting the
+    // environemnt variable $NoDefaultCurrentDirectoryInExePath
+    oldval = vim_getenv((char_u *)"NoDefaultCurrentDirectoryInExePath",
+           &must_free);
+    vim_setenv((char_u *)"NoDefaultCurrentDirectoryInExePath", (char_u *)"1");
 
     out_flush();
 
@@ -5552,6 +5574,8 @@ mch_call_shell(
            // Use a terminal window to run the command in.
            x = mch_call_shell_terminal(cmd, options);
            resettitle();
+           restore_env_var((char_u *)"NoDefaultCurrentDirectoryInExePath",
+                   oldval, must_free);
            return x;
        }
     }
@@ -5776,6 +5800,10 @@ mch_call_shell(
        }
     }
 
+    // Restore original value of NoDefaultCurrentDirectoryInExePath
+    restore_env_var((char_u *)"NoDefaultCurrentDirectoryInExePath",
+           oldval, must_free);
+
     if (tmode == TMODE_RAW)
     {
        // The shell may have messed with the mode, always set it.

diff --git a/src/testdir/test_terminal2.vim b/src/testdir/test_terminal2.vim
index dea2e1fc5110b23e98ce2c2b7de2059819837502..cb2ed5ebb995af43c56cfe27aaee2189c5dab15a 100644 (file)
--- a/src/testdir/test_terminal2.vim
+++ b/src/testdir/test_terminal2.vim
@@ -440,6 +440,14 @@ func Test_zz2_terminal_guioptions_bang()
   call writefile(contents, filename, 'D')
   call setfperm(filename, 'rwxrwx---')
 
+  if has("win32")
+    " should not execute anything below the current directory
+    let exitval = 1
+    execute printf(':!%s%s %d', prefix, filename, exitval)
+    call assert_equal(exitval, v:shell_error)
+    let prefix = '.\'
+  endif
+
   " Check if v:shell_error is equal to the exit status.
   let exitval = 0
   execute printf(':!%s%s %d', prefix, filename, exitval)
@@ -732,5 +740,40 @@ func Test_term_gettty()
   exe buf . 'bwipe'
 endfunc
 
+func Test_windows_external_cmd_in_cwd()
+  " Check that Vim does not execute anything from current directory
+  CheckMSWindows
+
+  " just in case
+  call system('rd /S /Q Xfolder')
+  call mkdir('Xfolder', 'R')
+  cd Xfolder
+
+  let contents = ['@echo off', 'echo filename1.txt:1:AAAA']
+  call writefile(contents, 'findstr.cmd')
+
+  let file1 = ['AAAA', 'THIS FILE SHOULD NOT BE FOUND']
+  let file2 = ['BBBB', 'THIS FILE SHOULD BE FOUND']
+
+  call writefile(file1, 'filename1.txt')
+  call writefile(file2, 'filename2.txt')
+
+  " use silent to avoid hit-enter-prompt
+  sil grep BBBB filename*.txt
+
+  call assert_equal('filename2.txt', @%)
+
+  let output = system('findstr BBBB filename*')
+  " Match trailing newline byte
+  call assert_match('filename2.txt:BBBB.', output)
+
+  set guioptions+=!
+
+  let output = system('findstr BBBB filename*')
+  call assert_match('filename2.txt:BBBB.', output)
+
+  cd -
+  set guioptions&
+endfunc
 
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 916bc21744f9ccfb52f784a810bfbbaddb2e0832..545ecd309cee662aa49ef929b5da2ddd079d2828 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -729,6 +729,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1947,
 /**/
     1946,
 /**/