From d2f2e2725c6f8d3263516e7fdf7655ef1c379172 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 06:55:08 +0800 Subject: [PATCH 1/7] vim-patch:9.0.1532: crash when expanding "~" in substitute causes very long text Problem: Crash when expanding "~" in substitute causes very long text. Solution: Limit the text length to MAXCOL. https://github.com/vim/vim/commit/ab9a2d884b3a4abe319606ea95a5a6d6b01cd73a Co-authored-by: Bram Moolenaar --- src/nvim/regexp.c | 39 ++++++++++++++++------------ test/old/testdir/test_substitute.vim | 14 ++++++++++ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/nvim/regexp.c b/src/nvim/regexp.c index 48cde447b5..9e08d2615b 100644 --- a/src/nvim/regexp.c +++ b/src/nvim/regexp.c @@ -1642,41 +1642,46 @@ static void do_lower(int *d, int c) char *regtilde(char *source, int magic, bool preview) { char *newsub = source; - char *tmpsub; - char *p; - int len; - int prevlen; - for (p = newsub; *p; p++) { + for (char *p = newsub; *p; p++) { if ((*p == '~' && magic) || (*p == '\\' && *(p + 1) == '~' && !magic)) { if (reg_prev_sub != NULL) { // length = len(newsub) - 1 + len(prev_sub) + 1 - prevlen = (int)strlen(reg_prev_sub); - tmpsub = xmalloc(strlen(newsub) + (size_t)prevlen); + // Avoid making the text longer than MAXCOL, it will cause + // trouble at some point. + size_t prevsublen = strlen(reg_prev_sub); + size_t newsublen = strlen(newsub); + if (prevsublen > MAXCOL || newsublen > MAXCOL + || newsublen + prevsublen > MAXCOL) { + emsg(_(e_resulting_text_too_long)); + break; + } + + char *tmpsub = xmalloc(newsublen + prevsublen); // copy prefix - len = (int)(p - newsub); // not including ~ - memmove(tmpsub, newsub, (size_t)len); + size_t prefixlen = (size_t)(p - newsub); // not including ~ + memmove(tmpsub, newsub, prefixlen); // interpret tilde - memmove(tmpsub + len, reg_prev_sub, (size_t)prevlen); + memmove(tmpsub + prefixlen, reg_prev_sub, prevsublen); // copy postfix if (!magic) { - p++; // back off backslash + p++; // back off backslash } - STRCPY(tmpsub + len + prevlen, p + 1); + STRCPY(tmpsub + prefixlen + prevsublen, p + 1); - if (newsub != source) { // already allocated newsub + if (newsub != source) { // allocated newsub before xfree(newsub); } newsub = tmpsub; - p = newsub + len + prevlen; + p = newsub + prefixlen + prevsublen; } else if (magic) { - STRMOVE(p, p + 1); // remove '~' + STRMOVE(p, p + 1); // remove '~' } else { - STRMOVE(p, p + 2); // remove '\~' + STRMOVE(p, p + 2); // remove '\~' } p--; } else { - if (*p == '\\' && p[1]) { // skip escaped characters + if (*p == '\\' && p[1]) { // skip escaped characters p++; } p += utfc_ptr2len(p) - 1; diff --git a/test/old/testdir/test_substitute.vim b/test/old/testdir/test_substitute.vim index 8dff0cda52..9f1adda3bf 100644 --- a/test/old/testdir/test_substitute.vim +++ b/test/old/testdir/test_substitute.vim @@ -1415,6 +1415,20 @@ func Test_substitute_short_cmd() bw! endfunc +" Check handling expanding "~" resulting in extremely long text. +func Test_substitute_tilde_too_long() + enew! + + s/.*/ixxx + s//~~~~~~~~~AAAAAAA@( + + " Either fails with "out of memory" or "text too long". + " This can take a long time. + call assert_fails('sil! norm &&&&&&&&&', ['E1240:\|E342:']) + + bwipe! +endfunc + " This should be done last to reveal a memory leak when vim_regsub_both() is " called to evaluate an expression but it is not used in a second call. func Test_z_substitute_expr_leak() From f6658a1e785a6b603db3166083bef9cc56b65487 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 07:05:37 +0800 Subject: [PATCH 2/7] vim-patch:9.0.1534: test for expanding "~" in substitute takes too long Problem: Test for expanding "~" in substitute takes too long. Solution: Disable the test for now. https://github.com/vim/vim/commit/916d6dd5b1834293e21a72ef70175aae57e78fba Co-authored-by: Bram Moolenaar --- test/old/testdir/test_substitute.vim | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/test/old/testdir/test_substitute.vim b/test/old/testdir/test_substitute.vim index 9f1adda3bf..04cbd36f87 100644 --- a/test/old/testdir/test_substitute.vim +++ b/test/old/testdir/test_substitute.vim @@ -1416,18 +1416,19 @@ func Test_substitute_short_cmd() endfunc " Check handling expanding "~" resulting in extremely long text. -func Test_substitute_tilde_too_long() - enew! - - s/.*/ixxx - s//~~~~~~~~~AAAAAAA@( - - " Either fails with "out of memory" or "text too long". - " This can take a long time. - call assert_fails('sil! norm &&&&&&&&&', ['E1240:\|E342:']) - - bwipe! -endfunc +" FIXME: disabled, it takes too long to run on CI +#func Test_substitute_tilde_too_long() +# enew! +# +# s/.*/ixxx +# s//~~~~~~~~~AAAAAAA@( +# +# " Either fails with "out of memory" or "text too long". +# " This can take a long time. +# call assert_fails('sil! norm &&&&&&&&&', ['E1240:\|E342:']) +# +# bwipe! +#endfunc " This should be done last to reveal a memory leak when vim_regsub_both() is " called to evaluate an expression but it is not used in a second call. From 354b57b01ff38fe994e4bcab464dd70f5fad14d0 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 07:06:28 +0800 Subject: [PATCH 3/7] vim-patch:9.0.1535: test commented out in a wrong way Problem: Test commented out in a wrong way. Solution: Use legacy script comment character. https://github.com/vim/vim/commit/a4467c433a767cc2dc046ff134094c1b6305b678 Co-authored-by: Bram Moolenaar --- test/old/testdir/test_substitute.vim | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/old/testdir/test_substitute.vim b/test/old/testdir/test_substitute.vim index 04cbd36f87..956d51893d 100644 --- a/test/old/testdir/test_substitute.vim +++ b/test/old/testdir/test_substitute.vim @@ -1417,18 +1417,18 @@ endfunc " Check handling expanding "~" resulting in extremely long text. " FIXME: disabled, it takes too long to run on CI -#func Test_substitute_tilde_too_long() -# enew! -# -# s/.*/ixxx -# s//~~~~~~~~~AAAAAAA@( -# -# " Either fails with "out of memory" or "text too long". -# " This can take a long time. -# call assert_fails('sil! norm &&&&&&&&&', ['E1240:\|E342:']) -# -# bwipe! -#endfunc +"func Test_substitute_tilde_too_long() +" enew! +" +" s/.*/ixxx +" s//~~~~~~~~~AAAAAAA@( +" +" " Either fails with "out of memory" or "text too long". +" " This can take a long time. +" call assert_fails('sil! norm &&&&&&&&&', ['E1240:\|E342:']) +" +" bwipe! +"endfunc " This should be done last to reveal a memory leak when vim_regsub_both() is " called to evaluate an expression but it is not used in a second call. From a4c111ae690448176ae584b9a2181923d1b2cdbd Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 06:41:03 +0800 Subject: [PATCH 4/7] vim-patch:9.0.2108: [security]: overflow with count for :s command Problem: [security]: overflow with count for :s command Solution: Abort the :s command if the count is too large If the count after the :s command is larger than what fits into a (signed) long variable, abort with e_value_too_large. Adds a test with INT_MAX as count and verify it correctly fails. It seems the return value on Windows using mingw compiler wraps around, so the initial test using :s/./b/9999999999999999999999999990 doesn't fail there, since the count is wrapping around several times and finally is no longer larger than 2147483647. So let's just use 2147483647 in the test, which hopefully will always cause a failure https://github.com/vim/vim/commit/ac63787734fda2e294e477af52b3bd601517fa78 Co-authored-by: Christian Brabandt --- runtime/doc/change.txt | 6 +++--- runtime/doc/cmdline.txt | 1 + src/nvim/cmdhist.c | 2 -- src/nvim/ex_cmds.c | 7 ++++++- src/nvim/globals.h | 6 ++++-- test/old/testdir/test_substitute.vim | 1 + 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/runtime/doc/change.txt b/runtime/doc/change.txt index 2c47421b02..e1bb7c5fc7 100644 --- a/runtime/doc/change.txt +++ b/runtime/doc/change.txt @@ -619,9 +619,9 @@ original user. current line only. When [count] is given, replace in [count] lines, starting with the last line in [range]. When [range] is omitted start in the current line. - *E939* - [count] must be a positive number. Also see - |cmdline-ranges|. + *E939* *E1510* + [count] must be a positive number (max 2147483647) + Also see |cmdline-ranges|. See |:s_flags| for [flags]. The delimiter doesn't need to be /, see diff --git a/runtime/doc/cmdline.txt b/runtime/doc/cmdline.txt index 291286d8f7..8bed8a9ffc 100644 --- a/runtime/doc/cmdline.txt +++ b/runtime/doc/cmdline.txt @@ -339,6 +339,7 @@ terminals) A positive number represents the absolute index of an entry as it is given in the first column of a :history listing. This number remains fixed even if other entries are deleted. + (see |E1510|) A negative number means the relative position of an entry, counted from the newest entry (which has index -1) backwards. diff --git a/src/nvim/cmdhist.c b/src/nvim/cmdhist.c index be94c08d3a..1f1d7d2eab 100644 --- a/src/nvim/cmdhist.c +++ b/src/nvim/cmdhist.c @@ -30,8 +30,6 @@ # include "cmdhist.c.generated.h" #endif -static const char e_val_too_large[] = N_("E1510: Value too large: %s"); - static histentry_T *(history[HIST_COUNT]) = { NULL, NULL, NULL, NULL, NULL }; static int hisidx[HIST_COUNT] = { -1, -1, -1, -1, -1 }; ///< lastused entry /// identifying (unique) number of newest history entry diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c index 521f8eb208..692b320335 100644 --- a/src/nvim/ex_cmds.c +++ b/src/nvim/ex_cmds.c @@ -3411,10 +3411,15 @@ static int do_sub(exarg_T *eap, const proftime_T timeout, const int cmdpreview_n // check for a trailing count cmd = skipwhite(cmd); if (ascii_isdigit(*cmd)) { - i = getdigits_int(&cmd, true, 0); + i = getdigits_int(&cmd, true, INT_MAX); if (i <= 0 && !eap->skip && subflags.do_error) { emsg(_(e_zerocount)); return 0; + } else if (i >= INT_MAX) { + char buf[20]; + vim_snprintf(buf, sizeof(buf), "%d", i); + semsg(_(e_val_too_large), buf); + return 0; } eap->line1 = eap->line2; eap->line2 += (linenr_T)i - 1; diff --git a/src/nvim/globals.h b/src/nvim/globals.h index 0cab3d1f8c..8e773d691a 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -1026,11 +1026,13 @@ EXTERN const char e_highlight_group_name_too_long[] INIT(= N_("E1249: Highlight EXTERN const char e_invalid_line_number_nr[] INIT(= N_("E966: Invalid line number: %ld")); -EXTERN char e_stray_closing_curly_str[] +EXTERN const char e_stray_closing_curly_str[] INIT(= N_("E1278: Stray '}' without a matching '{': %s")); -EXTERN char e_missing_close_curly_str[] +EXTERN const char e_missing_close_curly_str[] INIT(= N_("E1279: Missing '}': %s")); +EXTERN const char e_val_too_large[] INIT(= N_("E1510: Value too large: %s")); + EXTERN const char e_undobang_cannot_redo_or_move_branch[] INIT(= N_("E5767: Cannot use :undo! to redo or move to a different undo branch")); diff --git a/test/old/testdir/test_substitute.vim b/test/old/testdir/test_substitute.vim index 956d51893d..75062f13aa 100644 --- a/test/old/testdir/test_substitute.vim +++ b/test/old/testdir/test_substitute.vim @@ -206,6 +206,7 @@ func Test_substitute_count() call assert_equal(['foo foo', 'foo foo', 'foo foo', 'bar foo', 'bar foo'], \ getline(1, '$')) + call assert_fails('s/./b/2147483647', 'E1510:') bwipe! endfunc From 016c6fae2740781a4c62f382673de1f86732533a Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 07:11:56 +0800 Subject: [PATCH 5/7] vim-patch:9.0.2109: [security]: overflow in nv_z_get_count Problem: [security]: overflow in nv_z_get_count Solution: break out, if count is too large When getting the count for a normal z command, it may overflow for large counts given. So verify, that we can safely store the result in a long. https://github.com/vim/vim/commit/58f9befca1fa172068effad7f2ea5a9d6a7b0cca Co-authored-by: Christian Brabandt --- src/nvim/normal.c | 4 ++++ test/old/testdir/test_normal.vim | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/src/nvim/normal.c b/src/nvim/normal.c index f0f3d35468..a68d1098e5 100644 --- a/src/nvim/normal.c +++ b/src/nvim/normal.c @@ -2695,6 +2695,10 @@ static bool nv_z_get_count(cmdarg_T *cap, int *nchar_arg) if (nchar == K_DEL || nchar == K_KDEL) { n /= 10; } else if (ascii_isdigit(nchar)) { + if (n > INT_MAX / 10) { + clearopbeep(cap->oap); + break; + } n = n * 10 + (nchar - '0'); } else if (nchar == CAR) { win_setheight(n); diff --git a/test/old/testdir/test_normal.vim b/test/old/testdir/test_normal.vim index f5ef2cc4ca..6a8c15bd48 100644 --- a/test/old/testdir/test_normal.vim +++ b/test/old/testdir/test_normal.vim @@ -4164,4 +4164,9 @@ func Test_normal33_g_cmd_nonblank() bw! endfunc +func Test_normal34_zet_large() + " shouldn't cause overflow + norm! z9765405999999999999 +endfunc + " vim: shiftwidth=2 sts=2 expandtab From 809b05bf276892101895a713e1b8d1c209e5dfb7 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 07:14:07 +0800 Subject: [PATCH 6/7] vim-patch:9.0.2110: [security]: overflow in ex address parsing Problem: [security]: overflow in ex address parsing Solution: Verify that lnum is positive, before substracting from LONG_MAX [security]: overflow in ex address parsing When parsing relative ex addresses one may unintentionally cause an overflow (because LONG_MAX - lnum will overflow for negative addresses). So verify that lnum is actually positive before doing the overflow check. https://github.com/vim/vim/commit/060623e4a3bc72b011e7cd92bedb3bfb64e06200 Co-authored-by: Christian Brabandt --- src/nvim/ex_docmd.c | 2 +- test/old/testdir/test_excmd.vim | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index fee712bbed..0ca6e8bedb 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -3552,7 +3552,7 @@ static linenr_T get_address(exarg_T *eap, char **ptr, cmd_addr_T addr_type, int if (i == '-') { lnum -= n; } else { - if (n >= INT32_MAX - lnum) { + if (lnum >= 0 && n >= INT32_MAX - lnum) { *errormsg = _(e_line_number_out_of_range); goto error; } diff --git a/test/old/testdir/test_excmd.vim b/test/old/testdir/test_excmd.vim index 15c83709ad..c729ff4929 100644 --- a/test/old/testdir/test_excmd.vim +++ b/test/old/testdir/test_excmd.vim @@ -745,5 +745,9 @@ func Test_write_after_rename() bwipe! endfunc +" catch address lines overflow +func Test_ex_address_range_overflow() + call assert_fails(':--+foobar', 'E492:') +endfunc " vim: shiftwidth=2 sts=2 expandtab From 9d39ad63182cebe18f89152f2239ff8aeff58308 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Fri, 17 Nov 2023 07:18:12 +0800 Subject: [PATCH 7/7] vim-patch:9.0.2111: [security]: overflow in get_number Problem: [security]: overflow in get_number Solution: Return 0 when the count gets too large [security]: overflow in get_number When using the z= command, we may overflow the count with values larger than MAX_INT. So verify that we do not overflow and in case when an overflow is detected, simply return 0 https://github.com/vim/vim/commit/73b2d3790cad5694fc0ed0db2926e4220c48d968 Co-authored-by: Christian Brabandt --- src/nvim/input.c | 3 +++ test/old/testdir/test_spell.vim | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/src/nvim/input.c b/src/nvim/input.c index 2f5eb49ce0..d6ade22fdb 100644 --- a/src/nvim/input.c +++ b/src/nvim/input.c @@ -180,6 +180,9 @@ int get_number(int colon, int *mouse_used) ui_cursor_goto(msg_row, msg_col); int c = safe_vgetc(); if (ascii_isdigit(c)) { + if (n > INT_MAX / 10) { + return 0; + } n = n * 10 + c - '0'; msg_putchar(c); typed++; diff --git a/test/old/testdir/test_spell.vim b/test/old/testdir/test_spell.vim index b2fc40ee08..a19b64a7de 100644 --- a/test/old/testdir/test_spell.vim +++ b/test/old/testdir/test_spell.vim @@ -1081,6 +1081,15 @@ func Test_spell_compatible() call StopVimInTerminal(buf) endfunc +func Test_z_equal_with_large_count() + split + set spell + call setline(1, "ff") + norm 0z=337203685477580 + set nospell + bwipe! +endfunc + let g:test_data_aff1 = [ \"SET ISO8859-1", \"TRY esianrtolcdugmphbyfvkwjkqxz-\xEB\xE9\xE8\xEA\xEF\xEE\xE4\xE0\xE2\xF6\xFC\xFB'ESIANRTOLCDUGMPHBYFVKWJKQXZ",