@matheusbsbs / matheus-bagliano / commits / 81c2d4c3a5

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>

Patrick Steinhardt committed Dec 1, 2022 at 15:47 UTC 81c2d4c3a5ba0e6ab8c348708441fed170e63a82

2 files changed +21 -5

t/t4205-log-pretty-formats.sh

@@ -905,6 +905,13 @@ test_expect_success 'log --pretty with padding and preceding control chars' '

905 test_cmp expect actual

906 '

907

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 +

915 test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit message' '

916 # We only assert that this command does not crash. This needs to be

917 # executed with the address sanitizer to demonstrate failure.

utf8.c

+14 -5

@@ -377,6 +377,7 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,

377 dst = sb_dst.buf;

378

379 while (src < end) {

380 + int glyph_width;

381 char *old;

382 size_t n;

383

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

391 break;

392

393 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 */

396 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) {

406 if (subst) {

407 memcpy(dst, subst, subst_len);

408 dst += subst_len;

409 subst = NULL;

410 }

402 - w += n;

411 + w += glyph_width;

412 continue;

413 }

414 memcpy(dst, old, src - old);

415 dst += src - old;

407 - w += n;

416 + w += glyph_width;

417 }

418 strbuf_setlen(&sb_dst, dst - sb_dst.buf);

419 strbuf_swap(sb_src, &sb_dst);