fix: snprintf buffer overflow detected by -D_FORTIFY_SOURCE=3 (neovim…

…#22780) Problem: Wrong buffer size argument passed to snprintf() in set_cmdarg(): Thread no. 1 (24 frames) neovim#8 snprintf at /usr/include/bits/stdio2.h:54 neovim#9 set_cmdarg at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/eval.c:7044 neovim#10 apply_autocmds_group at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/autocmd.c:1843 neovim#11 apply_autocmds_exarg at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/autocmd.c:1549 neovim#12 readfile at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:617 neovim#13 buf_reload at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:5038 neovim#14 buf_check_timestamp at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:4952 neovim#15 check_timestamps at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:4678 neovim#16 ex_checktime at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_cmds2.c:765 neovim#17 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1620 neovim#18 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 neovim#19 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 neovim#20 ex_execute at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/eval.c:7727 neovim#21 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1620 neovim#22 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 neovim#23 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 neovim#24 do_ucmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/usercmd.c:1661 neovim#25 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1612 neovim#26 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 neovim#27 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 neovim#28 nv_colon at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/normal.c:4058 neovim#29 normal_execute at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/normal.c:1172 neovim#30 state_enter at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/state.c:88 neovim#31 normal_enter at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/normal.c:471 Solution: Subtract the offset from the buffer size. Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
folke · May 22, 2023 · 500bd9b · 500bd9b
1 parent 8e3cec1
commit 500bd9b
Showing 1 changed file with 60 additions and 21 deletions.
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
@@ -6771,20 +6771,18 @@ char *set_cmdarg(exarg_T *eap, char *oldarg)
 {
   char *oldval = vimvars[VV_CMDARG].vv_str;
   if (eap == NULL) {
-    xfree(oldval);
-    vimvars[VV_CMDARG].vv_str = oldarg;
-    return NULL;
+    goto error;
   }
 
   size_t len = 0;
   if (eap->force_bin == FORCE_BIN) {
-    len = 6;
+    len += 6;  // " ++bin"
   } else if (eap->force_bin == FORCE_NOBIN) {
-    len = 8;
+    len += 8;  // " ++nobin"
   }
 
   if (eap->read_edit) {
-    len += 7;
+    len += 7;  // " ++edit"
   }
 
   if (eap->force_ff != 0) {
@@ -6797,48 +6795,89 @@ char *set_cmdarg(exarg_T *eap, char *oldarg)
     len += 7 + 4;  // " ++bad=" + "keep" or "drop"
   }
   if (eap->mkdir_p != 0) {
-    len += 4;
+    len += 4;  // " ++p"
   }
 
   const size_t newval_len = len + 1;
   char *newval = xmalloc(newval_len);
+  size_t xlen = 0;
+  int rc = 0;
 
   if (eap->force_bin == FORCE_BIN) {
-    snprintf(newval, newval_len, " ++bin");
+    rc = snprintf(newval, newval_len, " ++bin");
   } else if (eap->force_bin == FORCE_NOBIN) {
-    snprintf(newval, newval_len, " ++nobin");
+    rc = snprintf(newval, newval_len, " ++nobin");
   } else {
     *newval = NUL;
   }
+  if (rc < 0) {
+    goto error;
+  }
+  xlen += (size_t)rc;
 
   if (eap->read_edit) {
-    STRCAT(newval, " ++edit");
+    rc = snprintf(newval + xlen, newval_len - xlen, " ++edit");
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   }
 
   if (eap->force_ff != 0) {
-    snprintf(newval + strlen(newval), newval_len, " ++ff=%s",
-             eap->force_ff == 'u' ? "unix" :
-             eap->force_ff == 'd' ? "dos" : "mac");
+    rc = snprintf(newval + xlen,
+                  newval_len - xlen,
+                  " ++ff=%s",
+                  eap->force_ff == 'u'   ? "unix"
+                  : eap->force_ff == 'd' ? "dos" : "mac");
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   }
   if (eap->force_enc != 0) {
-    snprintf(newval + strlen(newval), newval_len, " ++enc=%s",
-             eap->cmd + eap->force_enc);
+    rc = snprintf(newval + (xlen), newval_len - xlen, " ++enc=%s", eap->cmd + eap->force_enc);
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   }
+
   if (eap->bad_char == BAD_KEEP) {
-    STRCPY(newval + strlen(newval), " ++bad=keep");
+    rc = snprintf(newval + xlen, newval_len - xlen, " ++bad=keep");
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   } else if (eap->bad_char == BAD_DROP) {
-    STRCPY(newval + strlen(newval), " ++bad=drop");
+    rc = snprintf(newval + xlen, newval_len - xlen, " ++bad=drop");
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   } else if (eap->bad_char != 0) {
-    snprintf(newval + strlen(newval), newval_len, " ++bad=%c",
-             eap->bad_char);
+    rc = snprintf(newval + xlen, newval_len - xlen, " ++bad=%c", eap->bad_char);
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   }
 
-  if (eap->mkdir_p) {
-    snprintf(newval, newval_len, " ++p");
+  if (eap->mkdir_p != 0) {
+    rc = snprintf(newval + xlen, newval_len - xlen, " ++p");
+    if (rc < 0) {
+      goto error;
+    }
+    xlen += (size_t)rc;
   }
+  assert(xlen <= newval_len);
 
   vimvars[VV_CMDARG].vv_str = newval;
   return oldval;
+
+error:
+  xfree(oldval);
+  vimvars[VV_CMDARG].vv_str = oldarg;
+  return NULL;
 }
 
 /// Check if variable "name[len]" is a local variable or an argument.