patch 9.2.0316: [security]: command injection in netbeans interface via defineAnnoType

Problem:  [security]: The netbeans defineAnnoType command passes typeName, fg and bg
          unsanitized to coloncmd(), allowing a malicious server to inject
          arbitrary Ex commands via '|'. Similarly, specialKeys does not
          validate key tokens before building a map command.
Solution: Validate typeName, fg and bg against an allowlist of safe
          characters before passing them to coloncmd()

Github Advisory:
GHSA-mr87-rhgv-7pw6

Supported by AI

Signed-off-by: Christian Brabandt <[email protected]>

Author: chrisbra

runtime/doc/netbeans.txt

@@ -1,4 +1,4 @@
-*netbeans.txt*	For Vim version 9.2.  Last change: 2026 Feb 14
+*netbeans.txt*	For Vim version 9.2.  Last change: 2026 Mar 07
 
 
 		  VIM REFERENCE MANUAL	  by Gordon Prieur et al.
@@ -849,7 +849,7 @@ REJECT		Not used.
 These errors occur when a message violates the protocol:
 *E627* *E628* *E629* *E632* *E633* *E634* *E635* *E636*
 *E637* *E638* *E639* *E640* *E641* *E642* *E643* *E644* *E645* *E646*
-*E647* *E648* *E650* *E651* *E652*
+*E647* *E648* *E649* *E650* *E651* *E652*
 
 
 ==============================================================================

runtime/doc/tags

@@ -5298,6 +5298,7 @@ E645	netbeans.txt	/*E645*
 E646	netbeans.txt	/*E646*
 E647	netbeans.txt	/*E647*
 E648	netbeans.txt	/*E648*
+E649	netbeans.txt	/*E649*
 E65	pattern.txt	/*E65*
 E650	netbeans.txt	/*E650*
 E651	netbeans.txt	/*E651*

src/errors.h

@@ -1664,7 +1664,8 @@ EXTERN char e_invalid_buffer_identifier_in_setdot[]
 	INIT(= N_("E647: Invalid buffer identifier in setDot"));
 EXTERN char e_invalid_buffer_identifier_in_close[]
 	INIT(= N_("E648: Invalid buffer identifier in close"));
-// E649 unused
+EXTERN char e_invalid_identifier_in_defineannotype[]
+	INIT(= N_("E649: Invalid identifier name in defineAnnoType"));
 EXTERN char e_invalid_buffer_identifier_in_defineannotype[]
 	INIT(= N_("E650: Invalid buffer identifier in defineAnnoType"));
 EXTERN char e_invalid_buffer_identifier_in_addanno[]

src/netbeans.c

@@ -40,6 +40,11 @@
 #define GUARDEDOFFSET 1000000 // base for "guarded" sign id's
 #define MAX_COLOR_LENGTH 32 // max length of color name in defineAnnoType
 
+// Characters valid in a sign/highlight group name
+#define VALID_CHARS         (char_u *)"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
+#define VALID_SIGNNAME_CHARS    VALID_CHARS "_"
+#define VALID_COLOR_CHARS       VALID_CHARS "#"
+
 // The first implementation (working only with Netbeans) returned "1.1".  The
 // protocol implemented here also supports A-A-P.
 static char *ExtEdProtocolVersion = "2.5";
@@ -77,6 +82,22 @@ static int dosetvisible = FALSE;
 static int needupdate = 0;
 static int inAtomic = 0;
 
+/*
+ * Return TRUE if "str" contains only characters from "allowed".
+ * Used to validate NetBeans-supplied strings before interpolating them
+ * into Ex commands via coloncmd().
+ */
+    static int
+nb_is_safe_string(char_u *str, char_u *allowed)
+{
+    if (str == NULL)
+	return FALSE;
+    for (char_u *p = str; *p != NUL; p++)
+	if (vim_strchr(allowed, *p) == NULL)
+	    return FALSE;
+    return TRUE;
+}
+
 /*
  * Callback invoked when the channel is closed.
  */
@@ -1949,6 +1970,15 @@ nb_do_cmd(
 		VIM_CLEAR(typeName);
 		parse_error = TRUE;
 	    }
+	    else if (!nb_is_safe_string(typeName, VALID_SIGNNAME_CHARS) ||
+		    (*fg != NUL && !nb_is_safe_string(fg, VALID_COLOR_CHARS)) ||
+		    (*bg != NUL && !nb_is_safe_string(bg, VALID_COLOR_CHARS)))
+	    {
+		nbdebug(("    invalid chars in typeName/fg/bg in defineAnnoType\n"));
+		emsg(_(e_invalid_identifier_in_defineannotype));
+		VIM_CLEAR(typeName);
+		parse_error = TRUE;
+	    }
 	    else if (typeName != NULL && tooltip != NULL && glyphFile != NULL)
 		addsigntype(buf, typeNum, typeName, tooltip, glyphFile, fg, bg);
 
@@ -2321,10 +2351,24 @@ special_keys(char_u *args)
 
 	if (strlen(tok) + i < KEYBUFLEN)
 	{
-	    vim_strncpy((char_u *)&keybuf[i], (char_u *)tok, KEYBUFLEN - i - 1);
-	    vim_snprintf(cmdbuf, sizeof(cmdbuf),
-				 "<silent><%s> :nbkey %s<CR>", keybuf, keybuf);
-	    do_map(MAPTYPE_MAP, (char_u *)cmdbuf, MODE_NORMAL, FALSE);
+	    // Only allow alphanumeric and function-key name characters.
+	    // Reject anything else to prevent map command injection.
+	    int safe = TRUE;
+	    for (char_u *tp = (char_u *)tok; *tp != NUL; tp++)
+	    {
+		if (!ASCII_ISALNUM(*tp) && *tp != '-')
+		{
+		    safe = FALSE;
+		    break;
+		}
+	    }
+	    if (safe)
+	    {
+		vim_strncpy((char_u *)&keybuf[i], (char_u *)tok, KEYBUFLEN - i - 1);
+		vim_snprintf(cmdbuf, sizeof(cmdbuf),
+				"<silent><%s> :nbkey %s<CR>", keybuf, keybuf);
+		do_map(MAPTYPE_MAP, (char_u *)cmdbuf, MODE_NORMAL, FALSE);
+	    }
 	}
 	tok = strtok(NULL, " ");
     }

src/po/vim.pot

@@ -113,8 +113,8 @@ def process_msgs(self, msgbuf):
                   'endAtomic_Test' : '0:endAtomic!95\n',
                   'AnnoScale_Test' : "".join(['2:defineAnnoType!60 ' + str(i) + ' "s' + str(i) + '" "x" "=>" blue none\n' for i in range(2, 26)]),
                   'detach_Test' : '2:close!96\n1:close!97\nDETACH\n',
-                  'specialKeys_overflow_Test' : '0:specialKeys!200 "' + 'A'*80 + '-X"\n'
-
+                  'specialKeys_overflow_Test' : '0:specialKeys!200 "' + 'A'*80 + '-X"\n',
+                  'defineAnnoType_injection_Test': '1:defineAnnoType!1 "MySign guifg=red|call writefile([\'inject\'],\'Xinject\')|" "tooltip" "glyphFile" 1 2\n'
                 }
                 # execute the specified test
                 if cmd not in testmap:

src/testdir/test_netbeans.py

@@ -1024,4 +1024,42 @@ func Test_nb_specialKeys_overflow()
   call s:run_server('Nb_specialKeys_overflow')
 endfunc
 
+func Nb_defineAnnoType_injection(port)
+  call writefile([], "Xnetbeans", 'D')
+  let g:last = 0
+
+  exe 'nbstart :localhost:' .. a:port .. ':bunny'
+  call assert_true(has("netbeans_enabled"))
+  call WaitFor('len(ReadXnetbeans()) > (g:last + 2)')
+  let g:last += 3
+
+  split Xcmdbuf
+  let cmdbufnr = bufnr()
+  call WaitFor('len(ReadXnetbeans()) > (g:last + 2)')
+  let g:last += 3
+  hide
+
+  sleep 1m
+
+  call delete('Xinject')
+  call appendbufline(cmdbufnr, '$', 'defineAnnoType_injection_Test')
+  " E475 from :sign is expected — catch it before RunServer sees it.
+  " give it a bit of time to process it
+  try
+    sleep 500m
+  catch /E475/
+  catch /E649/
+  endtry
+
+  " Injected call must not have created this file
+  call assert_false(filereadable('Xinject'))
+  call delete('Xinject')
+  bwipe! Xcmdbuf
+  nbclose
+endfunc
+
+func Test_nb_defineAnnoType_injection()
+  call ch_log('Test_nb_defineAnnoType_injection')
+  call s:run_server('Nb_defineAnnoType_injection')
+endfunc
 " vim: shiftwidth=2 sts=2 expandtab

src/testdir/test_netbeans.vim

@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    316,
 /**/
     315,
 /**/