coverity/13686: Do not allow NUL byte in precondition regex

Before this commit it emitted e_spell_trunc in the first case and
treated file as completely valid on the second. While first is fine
(both errors are actually valid, though old error is probably better),
second results in incorrect regex used.

2 files changed, 82 insertions, 20 deletions

diff --git a/src/nvim/spellfile.c b/src/nvim/spellfile.c
index bbef1f5032..2c0db0694a 100644
--- a/src/nvim/spellfile.c
+++ b/src/nvim/spellfile.c
@@ -267,7 +267,7 @@
 #define SAL_REM_ACCENTS         4
 
 #define VIMSPELLMAGIC "VIMspell"  // string at start of Vim spell file
-#define VIMSPELLMAGICL 8
+#define VIMSPELLMAGICL (sizeof(VIMSPELLMAGIC) - 1)
 #define VIMSPELLVERSION 50
 
 // Section IDs.  Only renumber them when VIMSPELLVERSION changes!
@@ -516,7 +516,6 @@ spell_load_file (
   FILE        *fd;
   char_u buf[VIMSPELLMAGICL];
   char_u      *p;
-  int i;
   int n;
   int len;
   char_u      *save_sourcing_name = sourcing_name;
@@ -558,8 +557,9 @@ spell_load_file (
   sourcing_lnum = 0;
 
   // <HEADER>: <fileID>
-  for (i = 0; i < VIMSPELLMAGICL; ++i)
+  for (size_t i = 0; i < VIMSPELLMAGICL; i++) {
     buf[i] = getc(fd);                                  // <fileID>
+  }
   if (STRNCMP(buf, VIMSPELLMAGIC, VIMSPELLMAGICL) != 0) {
     EMSG(_("E757: This does not look like a spell file"));
     goto endFAIL;
@@ -983,35 +983,36 @@ static int read_charflags_section(FILE *fd)
 // Return SP_*ERROR flags.
 static int read_prefcond_section(FILE *fd, slang_T *lp)
 {
-  int cnt;
-  int i;
-  int n;
-  char_u      *p;
-  char_u buf[MAXWLEN + 1];
-
   // <prefcondcnt> <prefcond> ...
-  cnt = get2c(fd);                                      // <prefcondcnt>
-  if (cnt <= 0)
+  const int cnt = get2c(fd);  // <prefcondcnt>
+  if (cnt <= 0) {
     return SP_FORMERROR;
+  }
 
   lp->sl_prefprog = xcalloc(cnt, sizeof(regprog_T *));
   lp->sl_prefixcnt = cnt;
 
-  for (i = 0; i < cnt; ++i) {
+  for (int i = 0; i < cnt; ++i) {
     // <prefcond> : <condlen> <condstr>
-    n = getc(fd);                                       // <condlen>
-    if (n < 0 || n >= MAXWLEN)
+    const int n = getc(fd);  // <condlen>
+    if (n < 0 || n >= MAXWLEN) {
       return SP_FORMERROR;
+    }
 
     // When <condlen> is zero we have an empty condition.  Otherwise
     // compile the regexp program used to check for the condition.
     if (n > 0) {
-      buf[0] = '^';                 // always match at one position only
-      p = buf + 1;
-      while (n-- > 0)
-        *p++ = getc(fd);                                // <condstr>
-      *p = NUL;
-      lp->sl_prefprog[i] = vim_regcomp(buf, RE_MAGIC + RE_STRING);
+      char buf[MAXWLEN + 1];
+      buf[0] = '^';  // always match at one position only
+      const size_t read_byte = fread(buf + 1, 1, (size_t)n, fd);
+      if (read_byte != (size_t)n) {
+        return feof(fd) ? SP_FORMERROR : SP_OTHERERROR;
+      }
+      if (memchr(buf + 1, NUL, (size_t)n)) {
+        return SP_FORMERROR;
+      }
+      buf[n + 1] = NUL;
+      lp->sl_prefprog[i] = vim_regcomp((char_u *)buf, RE_MAGIC | RE_STRING);
     }
   }
   return 0;
diff --git a/test/functional/spell/spellfile_spec.lua b/test/functional/spell/spellfile_spec.lua
new file mode 100644
index 0000000000..88f757249e
--- /dev/null
+++ b/test/functional/spell/spellfile_spec.lua
@@ -0,0 +1,61 @@
+local helpers = require('test.functional.helpers')(after_each)
+local lfs = require('lfs')
+
+local eq = helpers.eq
+local clear = helpers.clear
+local meths = helpers.meths
+local exc_exec = helpers.exc_exec
+local write_file = helpers.write_file
+
+local testdir = 'Xtest-functional-spell-spellfile.d'
+
+describe('spellfile', function()
+  before_each(function()
+    clear()
+    lfs.mkdir(testdir)
+    lfs.mkdir(testdir .. '/spell')
+  end)
+  after_each(function()
+    lfs.rmdir(testdir)
+  end)
+  --                   ┌ Magic string (#VIMSPELLMAGIC)
+  --                   │       ┌ Spell file version (#VIMSPELLVERSION)
+  local spellheader = 'VIMspell\050'
+  it('errors out when prefcond section is truncated', function()
+    meths.set_option('runtimepath', testdir)
+    write_file(testdir .. '/spell/en.ascii.spl',
+    --                         ┌ Section identifier (#SN_PREFCOND)
+    --                         │   ┌ Section flags (#SNF_REQUIRED or zero)
+    --                         │   │   ┌ Section length (4 bytes, MSB first)
+    --                         │   │   │
+               spellheader .. '\003\001\000\000\000\003'
+    --             ┌ Number of regexes in section (2 bytes, MSB first)
+    --             │       ┌ Condition length (1 byte)
+    --             │       │   ┌ Condition regex (missing!)
+               .. '\000\001\001')
+    meths.set_option('spelllang', 'en')
+    eq('Vim(set):E759: Format error in spell file',
+       exc_exec('set spell'))
+  end)
+  it('errors out when prefcond regexp contains NUL byte', function()
+    meths.set_option('runtimepath', testdir)
+    write_file(testdir .. '/spell/en.ascii.spl',
+    --                         ┌ Section identifier (#SN_PREFCOND)
+    --                         │   ┌ Section flags (#SNF_REQUIRED or zero)
+    --                         │   │   ┌ Section length (4 bytes, MSB first)
+    --                         │   │   │
+               spellheader .. '\003\001\000\000\000\008'
+    --             ┌ Number of regexes in section (2 bytes, MSB first)
+    --             │       ┌ Condition length (1 byte)
+    --             │       │   ┌ Condition regex
+    --             │       │   │       ┌ End of sections marker
+               .. '\000\001\005ab\000cd\255'
+    --             ┌ LWORDTREE tree length (4 bytes)
+    --             │               ┌ KWORDTREE tree length (4 bytes)
+    --             │               │               ┌ PREFIXTREE tree length
+               .. '\000\000\000\000\000\000\000\000\000\000\000\000')
+    meths.set_option('spelllang', 'en')
+    eq('Vim(set):E759: Format error in spell file',
+       exc_exec('set spell'))
+  end)
+end)