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>

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index aac9e4ce6cd5107693a14172e64884831060d462..5c5b56596e81a733cc10b1cb5ddd730809ae43c1 100755 (executable)
--- 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 30c7787cfa9a56493425bf16a13f61fc7ae23c37..077daf4b20a76aef57b95767f1e8d77746fe71da 100644 (file)
--- 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);