CVE-2022-1735: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: Classic Buffer Overflow in GitHub repository vim/vim prior to 8.2.4969.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.8
Published: 2022-05-17T19:15Z
Repository: https://github.com/vim/vim
Language: C
Files changed: src/change.c, src/edit.c, src/misc2.c, src/proto/misc2.pro, src/testdir/test_visual.vim, src/version.c
Commit date: 2022-05-16 19:40:59 +0100
Commit message: patch 8.2.4969: changing text in Visual mode may cause invalid memory access

Problem:    Changing text in Visual mode may cause invalid memory access.
Solution:   Check the Visual position after making a change.

Diff: diff --git a/src/change.c b/src/change.c
index a9c3e19f4cf9..47411ca59bea 100644
--- a/src/change.c
+++ b/src/change.c
@@ -546,10 +546,13 @@ changed_common(
 	// The current window is always after the last change, so that "g,"
 	// takes you back to it.
 	curwin->w_changelistidx = curbuf->b_changelistlen;
     }
 
+    if (VIsual_active)
+	check_visual_pos();
+
     FOR_ALL_TAB_WINDOWS(tp, wp)
     {
 	if (wp->w_buffer == curbuf)
 	{
 #ifdef FEAT_FOLDING
diff --git a/src/edit.c b/src/edit.c
index 5b06131f9603..6e76971c350a 100644
--- a/src/edit.c
+++ b/src/edit.c
@@ -2539,20 +2539,12 @@ stop_insert(
 		    ++curwin->w_cursor.col;	// put cursor back on the NUL
 	    }
 
 	    // <C-S-Right> may have started Visual mode, adjust the position for
 	    // deleted characters.
-	    if (VIsual_active && VIsual.lnum == curwin->w_cursor.lnum)
-	    {
-		int len = (int)STRLEN(ml_get_curline());
-
-		if (VIsual.col > len)
-		{
-		    VIsual.col = len;
-		    VIsual.coladd = 0;
-		}
-	    }
+	    if (VIsual_active)
+		check_visual_pos();
 	}
     }
     did_ai = FALSE;
 #ifdef FEAT_SMARTINDENT
     did_si = FALSE;
diff --git a/src/misc2.c b/src/misc2.c
index 7840ea98f0f6..e03ca4936dca 100644
--- a/src/misc2.c
+++ b/src/misc2.c
@@ -620,10 +620,35 @@ check_cursor(void)
 {
     check_cursor_lnum();
     check_cursor_col();
 }
 
+/*
+ * Check if VIsual position is valid, correct it if not.
+ * Can be called when in Visual mode and a change has been made.
+ */
+    void
+check_visual_pos(void)
+{
+    if (VIsual.lnum > curbuf->b_ml.ml_line_count)
+    {
+	VIsual.lnum = curbuf->b_ml.ml_line_count;
+	VIsual.col = 0;
+	VIsual.coladd = 0;
+    }
+    else
+    {
+	int len = (int)STRLEN(ml_get(VIsual.lnum));
+
+	if (VIsual.col > len)
+	{
+	    VIsual.col = len;
+	    VIsual.coladd = 0;
+	}
+    }
+}
+
 #if defined(FEAT_TEXTOBJ) || defined(PROTO)
 /*
  * Make sure curwin->w_cursor is not on the NUL at the end of the line.
  * Allow it when in Visual mode and 'selection' is not "old".
  */
@@ -2414,11 +2439,11 @@ get_user_name(char_u *buf, int len)
     else
 	vim_strncpy(buf, username, len - 1);
     return OK;
 }
 
-#if defined(EXITFREE) || defined(PROTOS)
+#if defined(EXITFREE) || defined(PROTO)
 /*
  * Free the memory allocated by get_user_name()
  */
     void
 free_username(void)
diff --git a/src/proto/misc2.pro b/src/proto/misc2.pro
index fe0a4fbb5388..2728ffe64fe2 100644
--- a/src/proto/misc2.pro
+++ b/src/proto/misc2.pro
@@ -15,10 +15,11 @@ linenr_T get_cursor_rel_lnum(win_T *wp, linenr_T lnum);
 void check_pos(buf_T *buf, pos_T *pos);
 void check_cursor_lnum(void);
 void check_cursor_col(void);
 void check_cursor_col_win(win_T *win);
 void check_cursor(void);
+void check_visual_pos(void);
 void adjust_cursor_col(void);
 int leftcol_changed(void);
 int copy_option_part(char_u **option, char_u *buf, int maxlen, char *sep_chars);
 int vim_isspace(int x);
 int simplify_key(int key, int *modifiers);
diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index abc8265bdbd3..c3230623b523 100644
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -1294,10 +1294,20 @@ func Test_visual_block_append_invalid_char()
   call assert_equal(['	-   let xxx', 'xxxxx   -', 'xxxxxxxx-xxx'], getline(1, 3))
   bwipe!
   set isprint&
 endfunc
 
+func Test_visual_block_with_substitute()
+  " this was reading beyond the end of the line
+  new
+  norm a0)
+  sil! norm  O
+  s/)
+  sil! norm 
+  bwipe!
+endfunc
+
 func Test_visual_reselect_with_count()
   " this was causing an illegal memory access
   let lines =<< trim END
 
 
diff --git a/src/version.c b/src/version.c
index 7397e6fb71b8..485e25780b3a 100644
--- a/src/version.c
+++ b/src/version.c
@@ -744,10 +744,12 @@ static char *(features[]) =
 	NULL
 };
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    4969,
 /**/
     4968,
 /**/
     4967,
 /**/

Vulnerable code: 	    if (VIsual_active && VIsual.lnum == curwin->w_cursor.lnum)
	    {
		int len = (int)STRLEN(ml_get_curline());

		if (VIsual.col > len)
		{
		    VIsual.col = len;
		    VIsual.coladd = 0;
		}
	    }
#if defined(EXITFREE) || defined(PROTOS)
Fixed code:     if (VIsual_active)
	check_visual_pos();

	    if (VIsual_active)
		check_visual_pos();
/*
 * Check if VIsual position is valid, correct it if not.
 * Can be called when in Visual mode and a change has been made.
 */
    void
check_visual_pos(void)
{
    if (VIsual.lnum > curbuf->b_ml.ml_line_count)
    {
	VIsual.lnum = curbuf->b_ml.ml_line_count;
	VIsual.col = 0;
	VIsual.coladd = 0;
    }
    else
    {
	int len = (int)STRLEN(ml_get(VIsual.lnum));

	if (VIsual.col > len)
	{
	    VIsual.col = len;
	    VIsual.coladd = 0;
	}
    }
}

#if defined(EXITFREE) || defined(PROTO)
void check_visual_pos(void);
func Test_visual_block_with_substitute()
  " this was reading beyond the end of the line
  new
  norm a0)
  sil! norm  O
  s/)
  sil! norm 
  bwipe!
endfunc

/**/
    4969,