commit 9198c1f2b1ddecde22af918541e0de2a32f0f45a
author Christian Brabandt <cb@256bit.org> Thu Oct 26 21:29:32 2023 +0200
committer Christian Brabandt <cb@256bit.org> Thu Oct 26 21:29:32 2023 +0200
tree 2af602f979b00fea18542cd679191c320009f9b2
parent 5f5131d775bf9966976e39aa38b070036cbfe969

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>

src/cmdhist.c

diff --git a/src/cmdhist.c b/src/cmdhist.c
index d398ca7..96a9b3e 100644
--- a/src/cmdhist.c
+++ b/src/cmdhist.c
@@ -742,7 +742,10 @@
 	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;
     }
 

src/errors.h

diff --git a/src/errors.h b/src/errors.h
index 79a785e..72957d8 100644
--- a/src/errors.h
+++ b/src/errors.h
@@ -3560,3 +3560,5 @@
 	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"));

src/ex_getln.c

diff --git a/src/ex_getln.c b/src/ex_getln.c
index 9683e2e..8f0be52 100644
--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -4377,6 +4377,10 @@
     {
 	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 @@
 	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;

src/testdir/test_history.vim

diff --git a/src/testdir/test_history.vim b/src/testdir/test_history.vim
index bb6d671..482328a 100644
--- a/src/testdir/test_history.vim
+++ b/src/testdir/test_history.vim
@@ -254,4 +254,12 @@
   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

src/version.c

diff --git a/src/version.c b/src/version.c
index 1ae5c98..0284594 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2068,
+/**/
     2067,
 /**/
     2066,