From 9cc346119bee505e0be3827b35c573701a307001 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sat, 2 Dec 2023 10:10:25 +0800 Subject: [PATCH] vim-patch:9.0.2142: [security]: stack-buffer-overflow in option callback functions Problem: [security]: stack-buffer-overflow in option callback functions Solution: pass size of errbuf down the call stack, use snprintf() instead of sprintf() We pass the error buffer down to the option callback functions, but in some parts of the code, we simply use sprintf(buf) to write into the error buffer, which can overflow. So let's pass down the length of the error buffer and use sprintf(buf, size) instead. Reported by @henices, thanks! https://github.com/vim/vim/commit/b39b240c386a5a29241415541f1c99e2e6b8ce47 Co-authored-by: Christian Brabandt --- src/nvim/option.c | 4 ++-- src/nvim/option_defs.h | 1 + src/nvim/option_vars.h | 2 ++ src/nvim/optionstr.c | 2 +- test/functional/legacy/crash_spec.lua | 10 ++++++++++ test/functional/legacy/messages_spec.lua | 5 ----- test/old/testdir/crash/poc_did_set_langmap | 1 + test/old/testdir/test_crash.vim | 8 ++++++++ 8 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 test/old/testdir/crash/poc_did_set_langmap diff --git a/src/nvim/option.c b/src/nvim/option.c index 7a7cda2fa0..ba9d1262d4 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1455,7 +1455,7 @@ int do_set(char *arg, int opt_flags) } else { char *startarg = arg; // remember for error message const char *errmsg = NULL; - char errbuf[80]; + char errbuf[ERR_BUFLEN]; do_one_set_option(opt_flags, &arg, &did_show, errbuf, sizeof(errbuf), &errmsg); @@ -3845,7 +3845,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt int opt_idx = findoption(name); if (opt_idx < 0) { - snprintf(errbuf, IOSIZE, _(e_unknown_option2), name); + snprintf(errbuf, sizeof(errbuf), _(e_unknown_option2), name); return errbuf; } diff --git a/src/nvim/option_defs.h b/src/nvim/option_defs.h index b2e8081a08..6d0401f319 100644 --- a/src/nvim/option_defs.h +++ b/src/nvim/option_defs.h @@ -66,6 +66,7 @@ typedef struct { /// is parameterized, then the "os_errbuf" buffer is used to store the error /// message (when it is not NULL). char *os_errbuf; + /// length of the error buffer size_t os_errbuflen; void *os_win; diff --git a/src/nvim/option_vars.h b/src/nvim/option_vars.h index b0e9ff9434..66185940ca 100644 --- a/src/nvim/option_vars.h +++ b/src/nvim/option_vars.h @@ -938,6 +938,8 @@ enum { // Value for b_p_ul indicating the global value must be used. #define NO_LOCAL_UNDOLEVEL (-123456) +#define ERR_BUFLEN 80 + #define SB_MAX 100000 // Maximum 'scrollback' value. #define MAX_NUMBERWIDTH 20 // used for 'numberwidth' and 'statuscolumn' diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 0a7d77e817..544524dd42 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -442,7 +442,7 @@ int check_signcolumn(win_T *wp) const char *check_stl_option(char *s) { int groupdepth = 0; - static char errbuf[80]; + static char errbuf[ERR_BUFLEN]; while (*s) { // Check for valid keys after % sequences diff --git a/test/functional/legacy/crash_spec.lua b/test/functional/legacy/crash_spec.lua index 979ebd9e33..094bea253e 100644 --- a/test/functional/legacy/crash_spec.lua +++ b/test/functional/legacy/crash_spec.lua @@ -21,3 +21,13 @@ it('no crash when ending Visual mode close the window to switch to', function() command('wincmd p') assert_alive() end) + +it('no crash when truncating overlong message', function() + pcall(command, 'source test/old/testdir/crash/vim_msg_trunc_poc') + assert_alive() +end) + +it('no crash with very long option error message', function() + pcall(command, 'source test/old/testdir/crash/poc_did_set_langmap') + assert_alive() +end) diff --git a/test/functional/legacy/messages_spec.lua b/test/functional/legacy/messages_spec.lua index e0cc1dc79c..146b00acb0 100644 --- a/test/functional/legacy/messages_spec.lua +++ b/test/functional/legacy/messages_spec.lua @@ -801,9 +801,4 @@ describe('messages', function() ]]) os.remove('b.txt') end) - - it('no crash when truncating overlong message', function() - pcall(command, 'source test/old/testdir/crash/vim_msg_trunc_poc') - assert_alive() - end) end) diff --git a/test/old/testdir/crash/poc_did_set_langmap b/test/old/testdir/crash/poc_did_set_langmap new file mode 100644 index 0000000000..f77145b9d1 --- /dev/null +++ b/test/old/testdir/crash/poc_did_set_langmap @@ -0,0 +1 @@ +se lmap=°xÿ7sil;drlmap=°xÿ7sil;drmo: pm31 3" \ No newline at end of file diff --git a/test/old/testdir/test_crash.vim b/test/old/testdir/test_crash.vim index eef1731454..1d4f435e4d 100644 --- a/test/old/testdir/test_crash.vim +++ b/test/old/testdir/test_crash.vim @@ -142,6 +142,13 @@ func Test_crash1_2() \ ' && echo "crash 3: [OK]" >> '.. result .. "\") call TermWait(buf, 150) + let file = 'crash/poc_did_set_langmap' + let cmn_args = "%s -u NONE -i NONE -n -X -m -n -e -s -S %s -c ':qa!'" + let args = printf(cmn_args, vim, file) + call term_sendkeys(buf, args .. + \ ' ; echo "crash 4: [OK]" >> '.. result .. "\") + call TermWait(buf, 150) + " clean up exe buf .. "bw!" @@ -151,6 +158,7 @@ func Test_crash1_2() \ 'crash 1: [OK]', \ 'crash 2: [OK]', \ 'crash 3: [OK]', + \ 'crash 4: [OK]', \ ] call assert_equal(expected, getline(1, '$'))