patch 8.2.5046: vim_regsub() can overwrite the destination

Problem: vim_regsub() can overwrite the destination. Solution: Pass the destination length, give an error when it doesn't fit.
vim · May 30, 2022 · 4aaf3e7 · zeertzjq · May 31, 2022 · brammool
1 parent 10db31f
commit 4aaf3e7
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 37 deletions.
diff --git a/src/eval.c b/src/eval.c
@@ -6905,7 +6905,7 @@ do_string_sub(
 	     * - The substituted text.
 	     * - The text after the match.
 	     */
-	    sublen = vim_regsub(&regmatch, sub, expr, tail, FALSE, TRUE, FALSE);
+	    sublen = vim_regsub(&regmatch, sub, expr, tail, 0, REGSUB_MAGIC);
 	    if (ga_grow(&ga, (int)((end - tail) + sublen -
 			    (regmatch.endp[0] - regmatch.startp[0]))) == FAIL)
 	    {
@@ -6917,8 +6917,9 @@ do_string_sub(
 	    i = (int)(regmatch.startp[0] - tail);
 	    mch_memmove((char_u *)ga.ga_data + ga.ga_len, tail, (size_t)i);
 	    // add the substituted text
-	    (void)vim_regsub(&regmatch, sub, expr, (char_u *)ga.ga_data
-					  + ga.ga_len + i, TRUE, TRUE, FALSE);
+	    (void)vim_regsub(&regmatch, sub, expr,
+				  (char_u *)ga.ga_data + ga.ga_len + i, sublen,
+				  REGSUB_COPY | REGSUB_MAGIC);
 	    ga.ga_len += i + sublen - 1;
 	    tail = regmatch.endp[0];
 	    if (*tail == NUL)

diff --git a/src/ex_cmds.c b/src/ex_cmds.c
@@ -4419,7 +4419,9 @@ ex_substitute(exarg_T *eap)
 		// get length of substitution part
 		sublen = vim_regsub_multi(&regmatch,
 				    sub_firstlnum - regmatch.startpos[0].lnum,
-			       sub, sub_firstline, FALSE, magic_isset(), TRUE);
+			       sub, sub_firstline, 0,
+			       REGSUB_BACKSLASH
+				    | (magic_isset() ? REGSUB_MAGIC : 0));
 #ifdef FEAT_EVAL
 		--textlock;
 
@@ -4528,7 +4530,9 @@ ex_substitute(exarg_T *eap)
 #endif
 		(void)vim_regsub_multi(&regmatch,
 				    sub_firstlnum - regmatch.startpos[0].lnum,
-				      sub, new_end, TRUE, magic_isset(), TRUE);
+				      sub, new_end, sublen,
+				      REGSUB_COPY | REGSUB_BACKSLASH
+					 | (magic_isset() ? REGSUB_MAGIC : 0));
 #ifdef FEAT_EVAL
 		--textlock;
 #endif

diff --git a/src/proto/regexp.pro b/src/proto/regexp.pro
@@ -6,8 +6,8 @@ char_u *skip_regexp_ex(char_u *startp, int dirc, int magic, char_u **newp, int *
 reg_extmatch_T *ref_extmatch(reg_extmatch_T *em);
 void unref_extmatch(reg_extmatch_T *em);
 char_u *regtilde(char_u *source, int magic);
-int vim_regsub(regmatch_T *rmp, char_u *source, typval_T *expr, char_u *dest, int copy, int magic, int backslash);
-int vim_regsub_multi(regmmatch_T *rmp, linenr_T lnum, char_u *source, char_u *dest, int copy, int magic, int backslash);
+int vim_regsub(regmatch_T *rmp, char_u *source, typval_T *expr, char_u *dest, int destlen, int flags);
+int vim_regsub_multi(regmmatch_T *rmp, linenr_T lnum, char_u *source, char_u *dest, int destlen, int flags);
 char_u *reg_submatch(int no);
 list_T *reg_submatch_list(int no);
 int vim_regcomp_had_eol(void);

diff --git a/src/regexp.c b/src/regexp.c
@@ -1649,7 +1649,7 @@ cstrchr(char_u *s, int c)
  */
 typedef void (*(*fptr_T)(int *, int));
 
-static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int copy, int magic, int backslash);
+static int vim_regsub_both(char_u *source, typval_T *expr, char_u *dest, int destlen, int flags);
 
     static fptr_T
 do_upper(int *d, int c)
@@ -1822,13 +1822,14 @@ clear_submatch_list(staticList10_T *sl)
  * vim_regsub() - perform substitutions after a vim_regexec() or
  * vim_regexec_multi() match.
  *
- * If "copy" is TRUE really copy into "dest".
- * If "copy" is FALSE nothing is copied, this is just to find out the length
- * of the result.
+ * If "flags" has REGSUB_COPY really copy into "dest[destlen]".
+ * Oterwise nothing is copied, only compue the length of the result.
  *
- * If "backslash" is TRUE, a backslash will be removed later, need to double
- * them to keep them, and insert a backslash before a CR to avoid it being
- * replaced with a line break later.
+ * If "flags" has REGSUB_MAGIC then behave like 'magic' is set.
+ *
+ * If "flags" has REGSUB_BACKSLASH a backslash will be removed later, need to
+ * double them to keep them, and insert a backslash before a CR to avoid it
+ * being replaced with a line break later.
  *
  * Note: The matched text must not change between the call of
  * vim_regexec()/vim_regexec_multi() and vim_regsub()!  It would make the back
@@ -1842,9 +1843,8 @@ vim_regsub(
     char_u	*source,
     typval_T	*expr,
     char_u	*dest,
-    int		copy,
-    int		magic,
-    int		backslash)
+    int		destlen,
+    int		flags)
 {
     int		result;
     regexec_T	rex_save;
@@ -1860,7 +1860,7 @@ vim_regsub(
     rex.reg_maxline = 0;
     rex.reg_buf = curbuf;
     rex.reg_line_lbr = TRUE;
-    result = vim_regsub_both(source, expr, dest, copy, magic, backslash);
+    result = vim_regsub_both(source, expr, dest, destlen, flags);
 
     rex_in_use = rex_in_use_save;
     if (rex_in_use)
@@ -1875,9 +1875,8 @@ vim_regsub_multi(
     linenr_T	lnum,
     char_u	*source,
     char_u	*dest,
-    int		copy,
-    int		magic,
-    int		backslash)
+    int		destlen,
+    int		flags)
 {
     int		result;
     regexec_T	rex_save;
@@ -1894,7 +1893,7 @@ vim_regsub_multi(
     rex.reg_firstlnum = lnum;
     rex.reg_maxline = curbuf->b_ml.ml_line_count - lnum;
     rex.reg_line_lbr = FALSE;
-    result = vim_regsub_both(source, NULL, dest, copy, magic, backslash);
+    result = vim_regsub_both(source, NULL, dest, destlen, flags);
 
     rex_in_use = rex_in_use_save;
     if (rex_in_use)
@@ -1908,9 +1907,8 @@ vim_regsub_both(
     char_u	*source,
     typval_T	*expr,
     char_u	*dest,
-    int		copy,
-    int		magic,
-    int		backslash)
+    int		destlen,
+    int		flags)
 {
     char_u	*src;
     char_u	*dst;
@@ -1925,6 +1923,7 @@ vim_regsub_both(
 #ifdef FEAT_EVAL
     static char_u   *eval_result = NULL;
 #endif
+    int		copy = flags & REGSUB_COPY;
 
     // Be paranoid...
     if ((source == NULL && expr == NULL) || dest == NULL)
@@ -1945,8 +1944,8 @@ vim_regsub_both(
 #ifdef FEAT_EVAL
 	// To make sure that the length doesn't change between checking the
 	// length and copying the string, and to speed up things, the
-	// resulting string is saved from the call with "copy" == FALSE to the
-	// call with "copy" == TRUE.
+	// resulting string is saved from the call with "flags & REGSUB_COPY"
+	// == 0 to the // call with "flags & REGSUB_COPY" != 0.
 	if (copy)
 	{
 	    if (eval_result != NULL)
@@ -2054,7 +2053,7 @@ vim_regsub_both(
 			had_backslash = TRUE;
 		    }
 		}
-		if (had_backslash && backslash)
+		if (had_backslash && (flags & REGSUB_BACKSLASH))
 		{
 		    // Backslashes will be consumed, need to double them.
 		    s = vim_strsave_escaped(eval_result, (char_u *)"\\");
@@ -2077,11 +2076,11 @@ vim_regsub_both(
     else
       while ((c = *src++) != NUL)
       {
-	if (c == '&' && magic)
+	if (c == '&' && (flags & REGSUB_MAGIC))
 	    no = 0;
 	else if (c == '\\' && *src != NUL)
 	{
-	    if (*src == '&' && !magic)
+	    if (*src == '&' && !(flags & REGSUB_MAGIC))
 	    {
 		++src;
 		no = 0;
@@ -2115,6 +2114,11 @@ vim_regsub_both(
 		// Copy a special key as-is.
 		if (copy)
 		{
+		    if (dst + 3 > dest + destlen)
+		    {
+			iemsg("vim_regsub_both(): not enough space");
+			return 0;
+		    }
 		    *dst++ = c;
 		    *dst++ = *src++;
 		    *dst++ = *src++;
@@ -2141,10 +2145,17 @@ vim_regsub_both(
 
 		    // If "backslash" is TRUE the backslash will be removed
 		    // later.  Used to insert a literal CR.
-		    default:	if (backslash)
+		    default:	if (flags & REGSUB_BACKSLASH)
 				{
 				    if (copy)
+				    {
+					if (dst + 1 > dest + destlen)
+					{
+					    iemsg("vim_regsub_both(): not enough space");
+					    return 0;
+					}
 					*dst = '\\';
+				    }
 				    ++dst;
 				}
 				c = *src++;
@@ -2166,10 +2177,18 @@ vim_regsub_both(
 	    if (has_mbyte)
 	    {
 		int totlen = mb_ptr2len(src - 1);
+		int charlen = mb_char2len(cc);
 
 		if (copy)
+		{
+		    if (dst + charlen > dest + destlen)
+		    {
+			iemsg("vim_regsub_both(): not enough space");
+			return 0;
+		    }
 		    mb_char2bytes(cc, dst);
-		dst += mb_char2len(cc) - 1;
+		}
+		dst += charlen - 1;
 		if (enc_utf8)
 		{
 		    int clen = utf_ptr2len(src - 1);
@@ -2179,15 +2198,29 @@ vim_regsub_both(
 		    if (clen < totlen)
 		    {
 			if (copy)
+			{
+			    if (dst + totlen - clen > dest + destlen)
+			    {
+				iemsg("vim_regsub_both(): not enough space");
+				return 0;
+			    }
 			    mch_memmove(dst + 1, src - 1 + clen,
 						     (size_t)(totlen - clen));
+			}
 			dst += totlen - clen;
 		    }
 		}
 		src += totlen - 1;
 	    }
 	    else if (copy)
-		    *dst = cc;
+	    {
+		if (dst + 1 > dest + destlen)
+		{
+		    iemsg("vim_regsub_both(): not enough space");
+		    return 0;
+		}
+		*dst = cc;
+	    }
 	    dst++;
 	}
 	else
@@ -2226,7 +2259,14 @@ vim_regsub_both(
 			    if (rex.reg_mmatch->endpos[no].lnum == clnum)
 				break;
 			    if (copy)
+			    {
+				if (dst + 1 > dest + destlen)
+				{
+				    iemsg("vim_regsub_both(): not enough space");
+				    return 0;
+				}
 				*dst = CAR;
+			    }
 			    ++dst;
 			    s = reg_getline(++clnum);
 			    if (rex.reg_mmatch->endpos[no].lnum == clnum)
@@ -2245,7 +2285,8 @@ vim_regsub_both(
 		    }
 		    else
 		    {
-			if (backslash && (*s == CAR || *s == '\\'))
+			if ((flags & REGSUB_BACKSLASH)
+						  && (*s == CAR || *s == '\\'))
 			{
 			    /*
 			     * Insert a backslash in front of a CR, otherwise
@@ -2255,6 +2296,11 @@ vim_regsub_both(
 			     */
 			    if (copy)
 			    {
+				if (dst + 2 > dest + destlen)
+				{
+				    iemsg("vim_regsub_both(): not enough space");
+				    return 0;
+				}
 				dst[0] = '\\';
 				dst[1] = *s;
 			    }
@@ -2279,6 +2325,7 @@ vim_regsub_both(
 			    if (has_mbyte)
 			    {
 				int l;
+				int charlen;
 
 				// Copy composing characters separately, one
 				// at a time.
@@ -2289,12 +2336,27 @@ vim_regsub_both(
 
 				s += l;
 				len -= l;
+				charlen = mb_char2len(cc);
 				if (copy)
+				{
+				    if (dst + charlen > dest + destlen)
+				    {
+					iemsg("vim_regsub_both(): not enough space");
+					return 0;
+				    }
 				    mb_char2bytes(cc, dst);
-				dst += mb_char2len(cc) - 1;
+				}
+				dst += charlen - 1;
 			    }
 			    else if (copy)
-				    *dst = cc;
+			    {
+				if (dst + 1 > dest + destlen)
+				{
+				    iemsg("vim_regsub_both(): not enough space");
+				    return 0;
+				}
+				*dst = cc;
+			    }
 			    dst++;
 			}
 
@@ -2711,7 +2773,7 @@ regprog_in_use(regprog_T *prog)
 
 /*
  * Match a regexp against a string.
- * "rmp->regprog" is a compiled regexp as returned by vim_regcomp().
+ * "rmp->regprog" must be a compiled regexp as returned by vim_regcomp().
  * Note: "rmp->regprog" may be freed and changed.
  * Uses curbuf for line count and 'iskeyword'.
  * When "nl" is TRUE consider a "\n" in "line" to be a line break.

diff --git a/src/regexp.h b/src/regexp.h
@@ -177,4 +177,9 @@ struct regengine
     //char_u	*expr;
 };
 
+// Flags used by vim_regsub() and vim_regsub_both()
+#define REGSUB_COPY	    1
+#define REGSUB_MAGIC	    2
+#define REGSUB_BACKSLASH    4
+
 #endif	// _REGEXP_H
diff --git a/src/version.c b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    5046,
 /**/
     5045,
 /**/