Skip to content

Commit 81c2d4c

Browse files
pks-tgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 937b71c commit 81c2d4c

File tree

2 files changed

+21
-5
lines changed

2 files changed

+21
-5
lines changed

t/t4205-log-pretty-formats.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,13 @@ test_expect_success 'log --pretty with padding and preceding control chars' '
905905
test_cmp expect actual
906906
'
907907

908+
test_expect_success 'log --pretty truncation with control chars' '
909+
test_commit "$(printf "\20\20\20\20xxxx")" file contents commit-with-control-chars &&
910+
printf "\20\20\20\20x.." >expect &&
911+
git log -1 --pretty="format:%<(3,trunc)%s" commit-with-control-chars >actual &&
912+
test_cmp expect actual
913+
'
914+
908915
test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit message' '
909916
# We only assert that this command does not crash. This needs to be
910917
# executed with the address sanitizer to demonstrate failure.

utf8.c

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
377377
dst = sb_dst.buf;
378378

379379
while (src < end) {
380+
int glyph_width;
380381
char *old;
381382
size_t n;
382383

@@ -390,21 +391,29 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
390391
break;
391392

392393
old = src;
393-
n = utf8_width((const char**)&src, NULL);
394-
if (!src) /* broken utf-8, do nothing */
394+
glyph_width = utf8_width((const char**)&src, NULL);
395+
if (!src) /* broken utf-8, do nothing */
395396
goto out;
396-
if (n && w >= pos && w < pos + width) {
397+
398+
/*
399+
* In case we see a control character we copy it into the
400+
* buffer, but don't add it to the width.
401+
*/
402+
if (glyph_width < 0)
403+
glyph_width = 0;
404+
405+
if (glyph_width && w >= pos && w < pos + width) {
397406
if (subst) {
398407
memcpy(dst, subst, subst_len);
399408
dst += subst_len;
400409
subst = NULL;
401410
}
402-
w += n;
411+
w += glyph_width;
403412
continue;
404413
}
405414
memcpy(dst, old, src - old);
406415
dst += src - old;
407-
w += n;
416+
w += glyph_width;
408417
}
409418
strbuf_setlen(&sb_dst, dst - sb_dst.buf);
410419
strbuf_swap(sb_src, &sb_dst);

0 commit comments

Comments
 (0)