diff options
author | Andreas Schneider <asn@cryptomilk.org> | 2023-03-26 01:22:14 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-26 08:22:14 +0800 |
commit | e3dab4b32609c63adfbb6bb425a4b19c1ff95cde (patch) | |
tree | 29134b6251101587bcf0c1cc152d17168dfeb784 | |
parent | 13eb6c2554653518098396e31fe04dfa82f5106f (diff) | |
download | rneovim-e3dab4b32609c63adfbb6bb425a4b19c1ff95cde.tar.gz rneovim-e3dab4b32609c63adfbb6bb425a4b19c1ff95cde.tar.bz2 rneovim-e3dab4b32609c63adfbb6bb425a4b19c1ff95cde.zip |
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 <asn@cryptomilk.org>
-rw-r--r-- | src/nvim/eval.c | 81 |
1 files 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. |