coverity/56795: Fix NULL dereference in :syn keyword non-printable

Bug was introduced 3 years earlier, in 13848aa: NULL keyword_copy was 
incorrectly treated as an indicator of OOM.

src/nvim/syntax.c

@@ -4246,83 +4246,81 @@ static void syn_cmd_keyword(exarg_T *eap, int syncing)
 
   if (rest != NULL) {
     syn_id = syn_check_group(arg, (int)(group_name_end - arg));
-    if (syn_id != 0)
+    if (syn_id != 0) {
-      /* allocate a buffer, for removing backslashes in the keyword */
+      // Allocate a buffer, for removing backslashes in the keyword.
       keyword_copy = xmalloc(STRLEN(rest) + 1);
-    syn_opt_arg.flags = 0;
-    syn_opt_arg.keyword = TRUE;
-    syn_opt_arg.sync_idx = NULL;
-    syn_opt_arg.has_cont_list = FALSE;
-    syn_opt_arg.cont_in_list = NULL;
-    syn_opt_arg.next_list = NULL;
-
-    /*
-     * The options given apply to ALL keywords, so all options must be
-     * found before keywords can be created.
-     * 1: collect the options and copy the keywords to keyword_copy.
-     */
-    cnt = 0;
-    p = keyword_copy;
-    for (; rest != NULL && !ends_excmd(*rest); rest = skipwhite(rest)) {
-      rest = get_syn_options(rest, &syn_opt_arg, &conceal_char);
-      if (rest == NULL || ends_excmd(*rest))
-        break;
-      /* Copy the keyword, removing backslashes, and add a NUL. */
-      while (*rest != NUL && !ascii_iswhite(*rest)) {
-        if (*rest == '\\' && rest[1] != NUL)
-          ++rest;
-        *p++ = *rest++;
-      }
-      *p++ = NUL;
-      ++cnt;
     }
+    if (keyword_copy != NULL) {
+      syn_opt_arg.flags = 0;
+      syn_opt_arg.keyword = true;
+      syn_opt_arg.sync_idx = NULL;
+      syn_opt_arg.has_cont_list = false;
+      syn_opt_arg.cont_in_list = NULL;
+      syn_opt_arg.next_list = NULL;
 
-    if (!eap->skip) {
+      // The options given apply to ALL keywords, so all options must be
-      /* Adjust flags for use of ":syn include". */
+      // found before keywords can be created.
-      syn_incl_toplevel(syn_id, &syn_opt_arg.flags);
+      // 1: collect the options and copy the keywords to keyword_copy.
-
+      cnt = 0;
-      /*
+      p = keyword_copy;
-       * 2: Add an entry for each keyword.
+      for (; rest != NULL && !ends_excmd(*rest); rest = skipwhite(rest)) {
-       */
+        rest = get_syn_options(rest, &syn_opt_arg, &conceal_char);
-      for (kw = keyword_copy; --cnt >= 0; kw += STRLEN(kw) + 1) {
+        if (rest == NULL || ends_excmd(*rest)) {
-        for (p = vim_strchr(kw, '[');; ) {
+          break;
-          if (p != NULL)
+        }
-            *p = NUL;
+        // Copy the keyword, removing backslashes, and add a NUL.
-          add_keyword(kw, syn_id, syn_opt_arg.flags,
+        while (*rest != NUL && !ascii_iswhite(*rest)) {
-              syn_opt_arg.cont_in_list,
+          if (*rest == '\\' && rest[1] != NUL) {
-              syn_opt_arg.next_list, conceal_char);
+            rest++;
-          if (p == NULL)
-            break;
-          if (p[1] == NUL) {
-            EMSG2(_("E789: Missing ']': %s"), kw);
-            goto error;
           }
-          if (p[1] == ']') {
+          *p++ = *rest++;
-            if (p[2] != NUL) {
+        }
-              EMSG3(_("E890: trailing char after ']': %s]%s"),
+        *p++ = NUL;
-                    kw, &p[2]);
+        cnt++;
+      }
+
+      if (!eap->skip) {
+        // Adjust flags for use of ":syn include".
+        syn_incl_toplevel(syn_id, &syn_opt_arg.flags);
+
+        // 2: Add an entry for each keyword.
+        for (kw = keyword_copy; --cnt >= 0; kw += STRLEN(kw) + 1) {
+          for (p = vim_strchr(kw, '[');; ) {
+            if (p != NULL) {
+              *p = NUL;
+            }
+            add_keyword(kw, syn_id, syn_opt_arg.flags,
+                        syn_opt_arg.cont_in_list,
+                        syn_opt_arg.next_list, conceal_char);
+            if (p == NULL) {
+              break;
+            }
+            if (p[1] == NUL) {
+              emsgf(_("E789: Missing ']': %s"), kw);
               goto error;
             }
-            kw = p + 1;
+            if (p[1] == ']') {
-            break;   // skip over the "]"
+              if (p[2] != NUL) {
-          }
+                emsgf(_("E890: trailing char after ']': %s]%s"),
-          if (has_mbyte) {
+                      kw, &p[2]);
-            int l = (*mb_ptr2len)(p + 1);
+                goto error;
+              }
+              kw = p + 1;
+              break;   // skip over the "]"
+            }
+            const int l = (*mb_ptr2len)(p + 1);
 
             memmove(p, p + 1, l);
             p += l;
-          } else {
-            p[0] = p[1];
-            ++p;
           }
         }
       }
-    }
 
 error:
-    xfree(keyword_copy);
+      xfree(keyword_copy);
-    xfree(syn_opt_arg.cont_in_list);
+      xfree(syn_opt_arg.cont_in_list);
-    xfree(syn_opt_arg.next_list);
+      xfree(syn_opt_arg.next_list);
+    }
   }
 
   if (rest != NULL)

test/functional/ex_cmds/syntax_spec.lua

@@ -0,0 +1,17 @@
+local helpers = require('test.functional.helpers')(after_each)
+
+local eq = helpers.eq
+local clear = helpers.clear
+local exc_exec = helpers.exc_exec
+
+describe(':syntax', function()
+  before_each(clear)
+
+  describe('keyword', function()
+    it('does not crash when group name contains unprintable characters',
+    function()
+      eq('Vim(syntax):E669: Unprintable character in group name',
+         exc_exec('syntax keyword \024 foo bar'))
+    end)
+  end)
+end)