utf8: refactor `strbuf_utf8_replace` to not rely on preallocated buffer

In `strbuf_utf8_replace`, we preallocate the destination buffer and then
use `memcpy` to copy bytes into it at computed offsets. This feels
rather fragile and is hard to understand at times. Refactor the code to
instead use `strbuf_add` and `strbuf_addstr` so that we can be sure that
there is no possibility to perform an out-of-bounds write.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 13 insertions, 21 deletions

diff --git a/utf8.c b/utf8.c
index 077daf4b20..d8a16af87c 100644
--- a/utf8.c
+++ b/utf8.c
@@ -365,26 +365,20 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,
 void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
 			 const char *subst)
 {
-	struct strbuf sb_dst = STRBUF_INIT;
-	char *src = sb_src->buf;
-	char *end = src + sb_src->len;
-	char *dst;
-	int w = 0, subst_len = 0;
+	const char *src = sb_src->buf, *end = sb_src->buf + sb_src->len;
+	struct strbuf dst;
+	int w = 0;
 
-	if (subst)
-		subst_len = strlen(subst);
-	strbuf_grow(&sb_dst, sb_src->len + subst_len);
-	dst = sb_dst.buf;
+	strbuf_init(&dst, sb_src->len);
 
 	while (src < end) {
+		const char *old;
 		int glyph_width;
-		char *old;
 		size_t n;
 
 		while ((n = display_mode_esc_sequence_len(src))) {
-			memcpy(dst, src, n);
+			strbuf_add(&dst, src, n);
 			src += n;
-			dst += n;
 		}
 
 		if (src >= end)
@@ -404,21 +398,19 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
 
 		if (glyph_width && w >= pos && w < pos + width) {
 			if (subst) {
-				memcpy(dst, subst, subst_len);
-				dst += subst_len;
+				strbuf_addstr(&dst, subst);
 				subst = NULL;
 			}
-			w += glyph_width;
-			continue;
+		} else {
+			strbuf_add(&dst, old, src - old);
 		}
-		memcpy(dst, old, src - old);
-		dst += src - old;
+
 		w += glyph_width;
 	}
-	strbuf_setlen(&sb_dst, dst - sb_dst.buf);
-	strbuf_swap(sb_src, &sb_dst);
+
+	strbuf_swap(sb_src, &dst);
 out:
-	strbuf_release(&sb_dst);
+	strbuf_release(&dst);
 }
 
 /*