patch 9.0.2068: [security] overflow in :history

Problem:  [security] overflow in :history
Solution: Check that value fits into int

The get_list_range() function, used to parse numbers for the :history
and :clist command internally uses long variables to store the numbers.
However function arguments are integer pointers, which can then
overflow.

Check that the return value from the vim_str2nr() function is not larger
than INT_MAX and if yes, bail out with an error. I guess nobody uses a
cmdline/clist history that needs so many entries... (famous last words).

It is only a moderate vulnerability, so impact should be low.

Github Advisory:
https://github.com/vim/vim/security/advisories/GHSA-q22m-h7m2-9mgm

Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/cmdhist.c b/src/cmdhist.c
index d398ca7a687b560938327cf65111d2096caedf00..96a9b3e95b86f18c00dd25013abec42bdbbee603 100644 (file)
--- a/src/cmdhist.c
+++ b/src/cmdhist.c
@@ -742,7 +742,10 @@ ex_history(exarg_T *eap)
        end = arg;
     if (!get_list_range(&end, &hisidx1, &hisidx2) || *end != NUL)
     {
-       semsg(_(e_trailing_characters_str), end);
+       if (*end != NUL)
+           semsg(_(e_trailing_characters_str), end);
+       else
+           semsg(_(e_val_too_large), arg);
        return;
     }
 

diff --git a/src/errors.h b/src/errors.h
index 79a785e1e2953c9c9a436970e53e117ed2e8e5cf..72957d8b93bdcdaa71f435cf23685bb3874b30cd 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -3560,3 +3560,5 @@ EXTERN char e_xattr_e2big[]
        INIT(= N_("E1508: Size of the extended attribute value is larger than the maximum size allowed"));
 EXTERN char e_xattr_other[]
        INIT(= N_("E1509: Error occurred when reading or writing extended attribute"));
+EXTERN char e_val_too_large[]
+       INIT(= N_("E1510: Value too large: %s"));

diff --git a/src/ex_getln.c b/src/ex_getln.c
index 9683e2ebd5af50819651287d002614cf15b5d394..8f0be520886bed3f59b62b2fa5c305541e024a31 100644 (file)
--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -4377,6 +4377,10 @@ get_list_range(char_u **str, int *num1, int *num2)
     {
        vim_str2nr(*str, NULL, &len, 0, &num, NULL, 0, FALSE, NULL);
        *str += len;
+       // overflow
+       if (num > INT_MAX)
+           return FAIL;
+
        *num1 = (int)num;
        first = TRUE;
     }
@@ -4387,8 +4391,12 @@ get_list_range(char_u **str, int *num1, int *num2)
        vim_str2nr(*str, NULL, &len, 0, &num, NULL, 0, FALSE, NULL);
        if (len > 0)
        {
-           *num2 = (int)num;
            *str = skipwhite(*str + len);
+           // overflow
+           if (num > INT_MAX)
+               return FAIL;
+
+           *num2 = (int)num;
        }
        else if (!first)                // no number given at all
            return FAIL;

diff --git a/src/testdir/test_history.vim b/src/testdir/test_history.vim
index bb6d67172585e67c135ad92ff2e3f8561ce60ac5..482328ab4aaefc3240533e50cc03d9d0b60b70f0 100644 (file)
--- a/src/testdir/test_history.vim
+++ b/src/testdir/test_history.vim
@@ -254,4 +254,12 @@ func Test_history_crypt_key()
   set key& bs& ts&
 endfunc
 
+" The following used to overflow and causing an use-after-free
+func Test_history_max_val()
+
+  set history=10
+  call assert_fails(':history 2147483648', 'E1510:')
+  set history&
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 1ae5c98714d1120f685080acaae01d7852098041..0284594f7653ce9fd4a4cfa30616ae91ce23d3c2 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2068,
 /**/
     2067,
 /**/