vim-patch:8.1.1366: using expressions in a modeline is unsafe

Problem:    Using expressions in a modeline is unsafe.
Solution:   Disallow using expressions in a modeline, unless the
            'modelineexpr' option is set.  Update help, add more tests.
110289e781

runtime/doc/options.txt

@@ -478,14 +478,17 @@ backslash in front of the ':' will be removed.  Example:
    /* vi:set dir=c\:\tmp: */ ~
 This sets the 'dir' option to "c:\tmp".  Only a single backslash before the
 ':' is removed.  Thus to include "\:" you have to specify "\\:".
-
+							*E992*
 No other commands than "set" are supported, for security reasons (somebody
 might create a Trojan horse text file with modelines).  And not all options
-can be set.  For some options a flag is set, so that when it's used the
-|sandbox| is effective.  Still, there is always a small risk that a modeline
-causes trouble.  E.g., when some joker sets 'textwidth' to 5 all your lines
-are wrapped unexpectedly.  So disable modelines before editing untrusted text.
-The mail ftplugin does this, for example.
+can be set.  For some options a flag is set, so that when the value is used
+the |sandbox| is effective.  Some options can only be set from the modeline
+when 'modelineexpr' is set (the default is off).
+
+Still, there is always a small risk that a modeline causes trouble.  E.g.,
+when some joker sets 'textwidth' to 5 all your lines are wrapped unexpectedly.
+So disable modelines before editing untrusted text.  The mail ftplugin does
+this, for example.
 
 Hint: If you would like to do something else than setting an option, you could
 define an autocommand that checks the file for a specific string.  For
@@ -2439,7 +2442,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	The expression will be evaluated in the |sandbox| if set from a
 	modeline, see |sandbox-option|.
 	This option can't be set from a |modeline| when the 'diff' option is
-	on.
+	on or the 'modelineexpr' option is off.
 
 	It is not allowed to change text or jump to another window while
 	evaluating 'foldexpr' |textlock|.
@@ -2554,6 +2557,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 
 	The expression will be evaluated in the |sandbox| if set from a
 	modeline, see |sandbox-option|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 	It is not allowed to change text or jump to another window while
 	evaluating 'foldtext' |textlock|.
@@ -2589,16 +2593,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	The expression will be evaluated in the |sandbox| when set from a
 	modeline, see |sandbox-option|.  That stops the option from working,
 	since changing the buffer text is not allowed.
-
-					*'formatoptions'* *'fo'*
-'formatoptions' 'fo'	string (default: "tcqj", Vi default: "vt")
-			local to buffer
-	This is a sequence of letters which describes how automatic
-	formatting is to be done.  See |fo-table|.  When the 'paste' option is
-	on, no formatting is done (like 'formatoptions' is empty).  Commas can
-	be inserted for readability.
-	To avoid problems with flags that are added in the future, use the
-	"+=" and "-=" feature of ":set" |add-option-flags|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
+	NOTE: This option is set to "" when 'compatible' is set.
 
 					*'formatlistpat'* *'flp'*
 'formatlistpat' 'flp'	string (default: "^\s*\d\+[\]:.)}\t ]\s*")
@@ -2613,6 +2609,16 @@ A jump table for the options with a short description can be found at |Q_op|.
 	The default recognizes a number, followed by an optional punctuation
 	character and white space.
 
+					*'formatoptions'* *'fo'*
+'formatoptions' 'fo'	string (default: "tcqj", Vi default: "vt")
+			local to buffer
+	This is a sequence of letters which describes how automatic
+	formatting is to be done.  See |fo-table|.  When the 'paste' option is
+	on, no formatting is done (like 'formatoptions' is empty).  Commas can
+	be inserted for readability.
+	To avoid problems with flags that are added in the future, use the
+	"+=" and "-=" feature of ":set" |add-option-flags|.
+
 						*'formatprg'* *'fp'*
 'formatprg' 'fp'	string (default "")
 			global or local to buffer |global-local|
@@ -2643,6 +2649,9 @@ A jump table for the options with a short description can be found at |Q_op|.
 	- system signals low battery life
 	- Nvim exits abnormally
 
+	This option cannot be set from a |modeline| or in the |sandbox|, for
+	security reasons.
+
 				   *'gdefault'* *'gd'* *'nogdefault'* *'nogd'*
 'gdefault' 'gd'		boolean	(default off)
 			global
@@ -2978,6 +2987,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	'guitabtooltip' is used for the tooltip, see below.
 	The expression will be evaluated in the |sandbox| when set from a
 	modeline, see |sandbox-option|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 	Only used when the GUI tab pages line is displayed.  'e' must be
 	present in 'guioptions'.  For the non-GUI tab pages line 'tabline' is
@@ -3106,6 +3116,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	When this option contains printf-style '%' items, they will be
 	expanded according to the rules used for 'statusline'.  See
 	'titlestring' for example settings.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 			*'ignorecase'* *'ic'* *'noignorecase'* *'noic'*
 'ignorecase' 'ic'	boolean	(default off)
@@ -3209,6 +3220,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 
 	The expression will be evaluated in the |sandbox| when set from a
 	modeline, see |sandbox-option|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 	It is not allowed to change text or jump to another window while
 	evaluating 'includeexpr' |textlock|.
@@ -3277,6 +3289,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 
 	The expression will be evaluated in the |sandbox| when set from a
 	modeline, see |sandbox-option|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 	It is not allowed to change text or jump to another window while
 	evaluating 'indentexpr' |textlock|.
@@ -3768,6 +3781,12 @@ A jump table for the options with a short description can be found at |Q_op|.
 <	This option cannot be set from a |modeline| or in the |sandbox|, for
 	security reasons.
 
+						*'makespellmem'* *'msm'*
+'makespellmem' 'msm'		string	(default "460000,2000,500")
+			global
+	Values relevant only when compressing a spell file, see |spell|.
+	This option cannot be set from a |modeline| or in the |sandbox|.
+
 						*'matchpairs'* *'mps'*
 'matchpairs' 'mps'	string	(default "(:),{:},[:]")
 			local to buffer
@@ -3883,6 +3902,17 @@ A jump table for the options with a short description can be found at |Q_op|.
 'modeline' 'ml'		boolean	(Vim default: on (off for root),
 				 Vi default: off)
 			local to buffer
+	If 'modeline' is on 'modelines' gives the number of lines that is
+	checked for set commands.  If 'modeline' is off or 'modelines' is zero
+	no lines are checked.  See |modeline|.
+
+			   *'modelineexpr'* *'mle'* *'nomodelineexpr'* *'nomle'*
+'modelineexpr' 'mle'	boolean (default: off)
+			global
+	When on allow some options that are an expression to be set in the
+	modeline.  Check the option for whether it is affected by
+	'modelineexpr'.  Also see |modeline|.
+
 						*'modelines'* *'mls'*
 'modelines' 'mls'	number	(default 5)
 			global
@@ -4622,6 +4652,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	When this option is not empty, it determines the content of the ruler
 	string, as displayed for the 'ruler' option.
 	The format of this option is like that of 'statusline'.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
+
 	The default ruler width is 17 characters.  To make the ruler 15
 	characters wide, put "%15(" at the start and "%)" at the end.
 	Example: >
@@ -5252,7 +5284,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 		"Pattern not found", "Back at original", etc.
 	  q	use "recording" instead of "recording @a"
 	  F	don't give the file info when editing a file, like `:silent`
-		was used for the command
+		was used for the command; note that this also affects messages
+		from autocommands
 
 	This gives you the opportunity to avoid that a change between buffers
 	requires you to hit <Enter>, but still gives as useful a message as
@@ -5773,6 +5806,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 
 	The 'statusline' option will be evaluated in the |sandbox| if set from
 	a modeline, see |sandbox-option|.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
 
 	It is not allowed to change text or jump to another window while
 	evaluating 'statusline' |textlock|.
@@ -5927,6 +5961,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	the text to be displayed.  Use "%1T" for the first label, "%2T" for
 	the second one, etc.  Use "%X" items for closing labels.
 
+	This option cannot be set in a modeline when 'modelineexpr' is off.
+
 	Keep in mind that only one of the tab pages is the current one, others
 	are invisible and you can't jump to their windows.
 
@@ -6203,8 +6239,11 @@ A jump table for the options with a short description can be found at |Q_op|.
 			global
 	When this option is not empty, it will be used for the title of the
 	window.  This happens only when the 'title' option is on.
+
 	When this option contains printf-style '%' items, they will be
 	expanded according to the rules used for 'statusline'.
+	This option cannot be set in a modeline when 'modelineexpr' is off.
+
 	Example: >
 	    :auto BufEnter * let &titlestring = hostname() . "/" . expand("%:p")
 	    :set title titlestring=%<%F%=%l/%L-%P titlelen=70
@@ -6238,6 +6277,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	undo file that exists is used.  When it cannot be read an error is
 	given, no further entry is used.
 	See |undo-persistence|.
+	This option cannot be set from a |modeline| or in the |sandbox|, for
+	security reasons.
 
 				*'undofile'* *'noundofile'* *'udf'* *'noudf'*
 'undofile' 'udf'	boolean	(default off)

src/nvim/generators/gen_options.lua

@@ -79,6 +79,7 @@ local get_flags = function(o)
     {'pri_mkrc'},
     {'deny_in_modelines', 'P_NO_ML'},
     {'deny_duplicates', 'P_NODUP'},
+    {'modelineexpr', 'P_MLE'},
   }) do
     local key_name = flag_desc[1]
     local def_name = flag_desc[2] or ('P_' .. key_name:upper())

src/nvim/option.c

@@ -251,6 +251,7 @@ typedef struct vimoption {
 #define P_RWINONLY     0x10000000U  ///< only redraw current window
 #define P_NDNAME       0x20000000U  ///< only normal dir name chars allowed
 #define P_UI_OPTION    0x40000000U  ///< send option to remote ui
+#define P_MLE          0x80000000U  ///< under control of 'modelineexpr'
 
 #define HIGHLIGHT_INIT \
   "8:SpecialKey,~:EndOfBuffer,z:TermCursor,Z:TermCursorNC,@:NonText," \
@@ -1290,6 +1291,11 @@ do_set (
           errmsg = (char_u *)_("E520: Not allowed in a modeline");
           goto skip;
         }
+        if ((flags & P_MLE) && !p_mle) {
+          errmsg = (char_u *)_(
+              "E992: Not allowed in a modeline when 'modelineexpr' is off");
+          goto skip;
+        }
         /* In diff mode some options are overruled.  This avoids that
          * 'foldmethod' becomes "marker" instead of "diff" and that
          * "wrap" gets set. */

src/nvim/option_defs.h

@@ -495,6 +495,7 @@ EXTERN long p_mmd;              // 'maxmapdepth'
 EXTERN long p_mmp;              // 'maxmempattern'
 EXTERN long p_mis;              // 'menuitems'
 EXTERN char_u   *p_msm;         // 'mkspellmem'
+EXTERN long p_mle;              // 'modelineexpr'
 EXTERN long p_mls;              // 'modelines'
 EXTERN char_u   *p_mouse;       // 'mouse'
 EXTERN char_u   *p_mousem;      // 'mousemodel'

src/nvim/options.lua

@@ -8,6 +8,7 @@
 --    defaults={condition=nil, if_true={vi=224, vim=0}, if_false=nil},
 --    secure=nil, gettext=nil, noglob=nil, normal_fname_chars=nil,
 --    pri_mkrc=nil, deny_in_modelines=nil, normal_dname_chars=nil,
+--    modelineexpr=nil,
 --    expand=nil, nodefault=nil, no_mkrc=nil, vi_def=true, vim=true,
 --    alloced=nil,
 --    save_pv_indir=nil,
@@ -286,6 +287,7 @@ return {
       deny_duplicates=true,
       vi_def=true,
       expand=true,
+      secure=true,
       varname='p_cdpath',
       defaults={if_true={vi=",,"}}
     },
@@ -856,6 +858,7 @@ return {
       type='string', scope={'window'},
       vi_def=true,
       vim=true,
+      modelineexpr=true,
       alloced=true,
       redraw={'current_window'},
       defaults={if_true={vi="0"}}
@@ -931,6 +934,7 @@ return {
       type='string', scope={'window'},
       vi_def=true,
       vim=true,
+      modelineexpr=true,
       alloced=true,
       redraw={'current_window'},
       defaults={if_true={vi="foldtext()"}}
@@ -940,6 +944,7 @@ return {
       type='string', scope={'buffer'},
       vi_def=true,
       vim=true,
+      modelineexpr=true,
       alloced=true,
       varname='p_fex',
       defaults={if_true={vi=""}}
@@ -1053,6 +1058,7 @@ return {
       full_name='guitablabel', abbreviation='gtl',
       type='string', scope={'global'},
       vi_def=true,
+      modelineexpr=true,
       redraw={'current_window'},
       enable_if=false,
     },
@@ -1143,6 +1149,7 @@ return {
       full_name='iconstring',
       type='string', scope={'global'},
       vi_def=true,
+      modelineexpr=true,
       varname='p_iconstring',
       defaults={if_true={vi=""}}
     },
@@ -1209,6 +1216,7 @@ return {
       full_name='includeexpr', abbreviation='inex',
       type='string', scope={'buffer'},
       vi_def=true,
+      modelineexpr=true,
       alloced=true,
       varname='p_inex',
       defaults={if_true={vi=""}}
@@ -1225,6 +1233,7 @@ return {
       type='string', scope={'buffer'},
       vi_def=true,
       vim=true,
+      modelineexpr=true,
       alloced=true,
       varname='p_inde',
       defaults={if_true={vi=""}}
@@ -1538,6 +1547,13 @@ return {
       varname='p_ml',
       defaults={if_true={vi=false, vim=true}}
     },
+    {
+      full_name='modelineexpr', abbreviation='mle',
+      type='bool', scope={'global'},
+      vi_def=true,
+      varname='p_mle',
+      defaults={if_true={vi=false}}
+    },
     {
       full_name='modelines', abbreviation='mls',
       type='number', scope={'global'},
@@ -1898,6 +1914,7 @@ return {
       type='string', scope={'global'},
       vi_def=true,
       alloced=true,
+      modelineexpr=true,
       redraw={'statuslines'},
       varname='p_ruf',
       defaults={if_true={vi=""}}
@@ -2293,6 +2310,7 @@ return {
       type='string', scope={'global', 'window'},
       vi_def=true,
       alloced=true,
+      modelineexpr=true,
       redraw={'statuslines'},
       varname='p_stl',
       defaults={if_true={vi=""}}
@@ -2352,6 +2370,7 @@ return {
       full_name='tabline', abbreviation='tal',
       type='string', scope={'global'},
       vi_def=true,
+      modelineexpr=true,
       redraw={'all_windows'},
       varname='p_tal',
       defaults={if_true={vi=""}}
@@ -2511,6 +2530,7 @@ return {
       full_name='titlestring',
       type='string', scope={'global'},
       vi_def=true,
+      modelineexpr=true,
       varname='p_titlestring',
       defaults={if_true={vi=""}}
     },

src/nvim/testdir/test49.in

@@ -4,7 +4,7 @@ If after adding a new test, the test output doesn't appear properly in
 test49.failed, try to add one or more "G"s at the line ending in "test.out"
 
 STARTTEST
-:se nomore
+:se nomore modelineexpr
 :lang mess C
 :so test49.vim
 :" Go back to this file and append the results from register r.

src/nvim/testdir/test_modeline.vim

@@ -60,14 +60,17 @@ func Test_modeline_keymap()
   set keymap= iminsert=0 imsearch=-1
 endfunc
 
-func s:modeline_fails(what, text)
+func s:modeline_fails(what, text, error)
+  if !exists('+' . a:what)
+    return
+  endif
   let fname = "Xmodeline_fails_" . a:what
   call writefile(['vim: set ' . a:text . ' :', 'nothing'], fname)
   let modeline = &modeline
   set modeline
   filetype plugin on
   syntax enable
-  call assert_fails('split ' . fname, 'E474:')
+  call assert_fails('split ' . fname, a:error)
   call assert_equal("", &filetype)
   call assert_equal("", &syntax)
 
@@ -79,16 +82,91 @@ func s:modeline_fails(what, text)
 endfunc
 
 func Test_modeline_filetype_fails()
-  call s:modeline_fails('filetype', 'ft=evil$CMD')
+  call s:modeline_fails('filetype', 'ft=evil$CMD', 'E474:')
 endfunc
 
 func Test_modeline_syntax_fails()
-  call s:modeline_fails('syntax', 'syn=evil$CMD')
+  call s:modeline_fails('syntax', 'syn=evil$CMD', 'E474:')
 endfunc
 
 func Test_modeline_keymap_fails()
-  if !has('keymap')
-    return
-  endif
-  call s:modeline_fails('keymap', 'keymap=evil$CMD')
+  call s:modeline_fails('keymap', 'keymap=evil$CMD', 'E474:')
+endfunc
+
+func Test_modeline_fails_always()
+  call s:modeline_fails('backupdir', 'backupdir=Something()', 'E520:')
+  call s:modeline_fails('cdpath', 'cdpath=Something()', 'E520:')
+  call s:modeline_fails('charconvert', 'charconvert=Something()', 'E520:')
+  call s:modeline_fails('completefunc', 'completefunc=Something()', 'E520:')
+  call s:modeline_fails('cscopeprg', 'cscopeprg=Something()', 'E520:')
+  call s:modeline_fails('diffexpr', 'diffexpr=Something()', 'E520:')
+  call s:modeline_fails('directory', 'directory=Something()', 'E520:')
+  call s:modeline_fails('equalprg', 'equalprg=Something()', 'E520:')
+  call s:modeline_fails('errorfile', 'errorfile=Something()', 'E520:')
+  call s:modeline_fails('exrc', 'exrc=Something()', 'E520:')
+  call s:modeline_fails('formatprg', 'formatprg=Something()', 'E520:')
+  call s:modeline_fails('fsync', 'fsync=Something()', 'E520:')
+  call s:modeline_fails('grepprg', 'grepprg=Something()', 'E520:')
+  call s:modeline_fails('helpfile', 'helpfile=Something()', 'E520:')
+  call s:modeline_fails('imactivatefunc', 'imactivatefunc=Something()', 'E520:')
+  call s:modeline_fails('imstatusfunc', 'imstatusfunc=Something()', 'E520:')
+  call s:modeline_fails('imstyle', 'imstyle=Something()', 'E520:')
+  call s:modeline_fails('keywordprg', 'keywordprg=Something()', 'E520:')
+  call s:modeline_fails('langmap', 'langmap=Something()', 'E520:')
+  call s:modeline_fails('luadll', 'luadll=Something()', 'E520:')
+  call s:modeline_fails('makeef', 'makeef=Something()', 'E520:')
+  call s:modeline_fails('makeprg', 'makeprg=Something()', 'E520:')
+  call s:modeline_fails('makespellmem', 'makespellmem=Something()', 'E520:')
+  call s:modeline_fails('mzschemedll', 'mzschemedll=Something()', 'E520:')
+  call s:modeline_fails('mzschemegcdll', 'mzschemegcdll=Something()', 'E520:')
+  call s:modeline_fails('omnifunc', 'omnifunc=Something()', 'E520:')
+  call s:modeline_fails('operatorfunc', 'operatorfunc=Something()', 'E520:')
+  call s:modeline_fails('perldll', 'perldll=Something()', 'E520:')
+  call s:modeline_fails('printdevice', 'printdevice=Something()', 'E520:')
+  call s:modeline_fails('patchexpr', 'patchexpr=Something()', 'E520:')
+  call s:modeline_fails('printexpr', 'printexpr=Something()', 'E520:')
+  call s:modeline_fails('pythondll', 'pythondll=Something()', 'E520:')
+  call s:modeline_fails('pythonhome', 'pythondll=Something()', 'E520:')
+  call s:modeline_fails('pythonthreedll', 'pythonthreedll=Something()', 'E520:')
+  call s:modeline_fails('pythonthreehome', 'pythonthreehome=Something()', 'E520:')
+  call s:modeline_fails('pyxversion', 'pyxversion=Something()', 'E520:')
+  call s:modeline_fails('rubydll', 'rubydll=Something()', 'E520:')
+  call s:modeline_fails('runtimepath', 'runtimepath=Something()', 'E520:')
+  call s:modeline_fails('secure', 'secure=Something()', 'E520:')
+  call s:modeline_fails('shell', 'shell=Something()', 'E520:')
+  call s:modeline_fails('shellcmdflag', 'shellcmdflag=Something()', 'E520:')
+  call s:modeline_fails('shellpipe', 'shellpipe=Something()', 'E520:')
+  call s:modeline_fails('shellquote', 'shellquote=Something()', 'E520:')
+  call s:modeline_fails('shellredir', 'shellredir=Something()', 'E520:')
+  call s:modeline_fails('shellxquote', 'shellxquote=Something()', 'E520:')
+  call s:modeline_fails('spellfile', 'spellfile=Something()', 'E520:')
+  call s:modeline_fails('spellsuggest', 'spellsuggest=Something()', 'E520:')
+  call s:modeline_fails('tcldll', 'tcldll=Something()', 'E520:')
+  call s:modeline_fails('titleold', 'titleold=Something()', 'E520:')
+  call s:modeline_fails('viewdir', 'viewdir=Something()', 'E520:')
+  call s:modeline_fails('viminfo', 'viminfo=Something()', 'E520:')
+  call s:modeline_fails('viminfofile', 'viminfofile=Something()', 'E520:')
+  call s:modeline_fails('winptydll', 'winptydll=Something()', 'E520:')
+  call s:modeline_fails('undodir', 'undodir=Something()', 'E520:')
+  " only check a few terminal options
+  " Skip these since nvim doesn't support termcodes as options
+  "call s:modeline_fails('t_AB', 't_AB=Something()', 'E520:')
+  "call s:modeline_fails('t_ce', 't_ce=Something()', 'E520:')
+  "call s:modeline_fails('t_sr', 't_sr=Something()', 'E520:')
+  "call s:modeline_fails('t_8b', 't_8b=Something()', 'E520:')
+endfunc
+
+func Test_modeline_fails_modelineexpr()
+  call s:modeline_fails('balloonexpr', 'balloonexpr=Something()', 'E992:')
+  call s:modeline_fails('foldexpr', 'foldexpr=Something()', 'E992:')
+  call s:modeline_fails('foldtext', 'foldtext=Something()', 'E992:')
+  call s:modeline_fails('formatexpr', 'formatexpr=Something()', 'E992:')
+  call s:modeline_fails('guitablabel', 'guitablabel=Something()', 'E992:')
+  call s:modeline_fails('iconstring', 'iconstring=Something()', 'E992:')
+  call s:modeline_fails('includeexpr', 'includeexpr=Something()', 'E992:')
+  call s:modeline_fails('indentexpr', 'indentexpr=Something()', 'E992:')
+  call s:modeline_fails('rulerformat', 'rulerformat=Something()', 'E992:')
+  call s:modeline_fails('statusline', 'statusline=Something()', 'E992:')
+  call s:modeline_fails('tabline', 'tabline=Something()', 'E992:')
+  call s:modeline_fails('titlestring', 'titlestring=Something()', 'E992:')
 endfunc