96 lines
3.1 KiB
Diff
96 lines
3.1 KiB
Diff
From 81c2d4c3a5ba0e6ab8c348708441fed170e63a82 Mon Sep 17 00:00:00 2001
|
|
From: Patrick Steinhardt <ps@pks.im>
|
|
Date: Thu, 1 Dec 2022 15:47:10 +0100
|
|
Subject: [PATCH] utf8: fix checking for glyph width in `strbuf_utf8_replace()`
|
|
|
|
In `strbuf_utf8_replace()`, we call `utf8_width()` to compute the width
|
|
of the current glyph. If the glyph is a control character though it can
|
|
be that `utf8_width()` returns `-1`, but because we assign this value to
|
|
a `size_t` the conversion will cause us to underflow. This bug can
|
|
easily be triggered with the following command:
|
|
|
|
$ git log --pretty='format:xxx%<|(1,trunc)%x10'
|
|
|
|
>From all I can see though this seems to be a benign underflow that has
|
|
no security-related consequences.
|
|
|
|
Fix the bug by using an `int` instead. When we see a control character,
|
|
we now copy it into the target buffer but don't advance the current
|
|
width of the string.
|
|
|
|
Signed-off-by: Patrick Steinhardt <ps@pks.im>
|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
---
|
|
t/t4205-log-pretty-formats.sh | 7 +++++++
|
|
utf8.c | 19 ++++++++++++++-----
|
|
2 files changed, 21 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
|
|
index aac9e4ce6c..5c5b56596e 100755
|
|
--- a/t/t4205-log-pretty-formats.sh
|
|
+++ b/t/t4205-log-pretty-formats.sh
|
|
@@ -905,6 +905,13 @@ test_expect_success 'log --pretty with padding and preceding control chars' '
|
|
test_cmp expect actual
|
|
'
|
|
|
|
+test_expect_success 'log --pretty truncation with control chars' '
|
|
+ test_commit "$(printf "\20\20\20\20xxxx")" file contents commit-with-control-chars &&
|
|
+ printf "\20\20\20\20x.." >expect &&
|
|
+ git log -1 --pretty="format:%<(3,trunc)%s" commit-with-control-chars >actual &&
|
|
+ test_cmp expect actual
|
|
+'
|
|
+
|
|
test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit message' '
|
|
# We only assert that this command does not crash. This needs to be
|
|
# executed with the address sanitizer to demonstrate failure.
|
|
diff --git a/utf8.c b/utf8.c
|
|
index 30c7787cfa..077daf4b20 100644
|
|
--- a/utf8.c
|
|
+++ b/utf8.c
|
|
@@ -377,6 +377,7 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
|
|
dst = sb_dst.buf;
|
|
|
|
while (src < end) {
|
|
+ int glyph_width;
|
|
char *old;
|
|
size_t n;
|
|
|
|
@@ -390,21 +391,29 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
|
|
break;
|
|
|
|
old = src;
|
|
- n = utf8_width((const char**)&src, NULL);
|
|
- if (!src) /* broken utf-8, do nothing */
|
|
+ glyph_width = utf8_width((const char**)&src, NULL);
|
|
+ if (!src) /* broken utf-8, do nothing */
|
|
goto out;
|
|
- if (n && w >= pos && w < pos + width) {
|
|
+
|
|
+ /*
|
|
+ * In case we see a control character we copy it into the
|
|
+ * buffer, but don't add it to the width.
|
|
+ */
|
|
+ if (glyph_width < 0)
|
|
+ glyph_width = 0;
|
|
+
|
|
+ if (glyph_width && w >= pos && w < pos + width) {
|
|
if (subst) {
|
|
memcpy(dst, subst, subst_len);
|
|
dst += subst_len;
|
|
subst = NULL;
|
|
}
|
|
- w += n;
|
|
+ w += glyph_width;
|
|
continue;
|
|
}
|
|
memcpy(dst, old, src - old);
|
|
dst += src - old;
|
|
- w += n;
|
|
+ w += glyph_width;
|
|
}
|
|
strbuf_setlen(&sb_dst, dst - sb_dst.buf);
|
|
strbuf_swap(sb_src, &sb_dst);
|
|
--
|
|
2.27.0
|
|
|