From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v2] utf8.c: fix strbuf_utf8_replace() consuming data beyond input string
Date: Sun, 10 Aug 2014 14:05:21 +0700 [thread overview]
Message-ID: <1407654321-7509-1-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1406639429-28748-1-git-send-email-pclouds@gmail.com>
The main loop in strbuf_utf8_replace() could summed up as:
while ('src' is still valid) {
1) advance 'src' to copy ANSI escape sequences
2) advance 'src' to copy/replace visible characters
}
The problem is after #1, 'src' may have reached the end of the string
(so 'src' points to NUL) and #2 will continue to copy that NUL as if
it's a normal character. Because the output is stored in a strbuf,
this NULL accounted in the 'len' field as well. Check after #1 and
break the loop if necessary.
The test does not look obvious, but the combination of %>>() should
make a call trace like this
show_log()
pretty_print_commit()
format_commit_message()
strbuf_expand()
format_commit_item()
format_and_pad_commit()
strbuf_utf8_replace()
where %C(auto)%d would insert a color reset escape sequence in the end
of the string given to strbuf_utf8_replace() and show_log() uses
fwrite() to send everything to stdout (including the incorrect NUL
inserted by strbuf_utf8_replace)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
t/t4205-log-pretty-formats.sh | 7 +++++++
utf8.c | 3 +++
2 files changed, 10 insertions(+)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 349c531..de0cc4a 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -431,6 +431,13 @@ EOF
test_cmp expected actual
'
+test_expect_success 'strbuf_utf8_replace() not producing NUL' '
+ git log --color --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)%C(auto)%d" |
+ test_decode_color |
+ nul_to_q >actual &&
+ ! grep Q actual
+'
+
# get new digests (with no abbreviations)
head1=$(git rev-parse --verify HEAD~0) &&
head2=$(git rev-parse --verify HEAD~1) &&
diff --git a/utf8.c b/utf8.c
index b30790d..401a6a5 100644
--- a/utf8.c
+++ b/utf8.c
@@ -382,6 +382,9 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
dst += n;
}
+ if (src >= end)
+ break;
+
old = src;
n = utf8_width((const char**)&src, NULL);
if (!src) /* broken utf-8, do nothing */
--
2.1.0.rc0.78.gc0d8480
prev parent reply other threads:[~2014-08-10 7:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-29 13:10 [PATCH] utf8.c: fix strbuf_utf8_replace copying the last NUL to dst string Nguyễn Thái Ngọc Duy
2014-07-29 19:56 ` Junio C Hamano
2014-07-30 10:20 ` Duy Nguyen
2014-07-30 18:23 ` Junio C Hamano
2014-08-10 7:05 ` Nguyễn Thái Ngọc Duy [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1407654321-7509-1-git-send-email-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.