From e3dab4b32609c63adfbb6bb425a4b19c1ff95cde Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Sun, 26 Mar 2023 01:22:14 +0100 Subject: fix: snprintf buffer overflow detected by -D_FORTIFY_SOURCE=3 (#22780) Problem: Wrong buffer size argument passed to snprintf() in set_cmdarg(): Thread no. 1 (24 frames) #8 snprintf at /usr/include/bits/stdio2.h:54 #9 set_cmdarg at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/eval.c:7044 #10 apply_autocmds_group at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/autocmd.c:1843 #11 apply_autocmds_exarg at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/autocmd.c:1549 #12 readfile at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:617 #13 buf_reload at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:5038 #14 buf_check_timestamp at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:4952 #15 check_timestamps at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/fileio.c:4678 #16 ex_checktime at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_cmds2.c:765 #17 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1620 #18 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 #19 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 #20 ex_execute at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/eval.c:7727 #21 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1620 #22 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 #23 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 #24 do_ucmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/usercmd.c:1661 #25 execute_cmd0 at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:1612 #26 do_one_cmd at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:2275 #27 do_cmdline at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/ex_docmd.c:584 #28 nv_colon at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/normal.c:4058 #29 normal_execute at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/normal.c:1172 #30 state_enter at /usr/src/debug/neovim-0.8.2-2.fc38.x86_64/src/nvim/state.c:88 #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 --- src/nvim/eval.c | 81 ++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 14f2379504..fc9ddb75ef 100644 --- 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. -- cgit